ACPI: ACPI documentations and trivial fixes
Subsystems (5)
cover ACPI: ACPI documentations and trivial fixes
This patch adds ACPI documentations related to:
1. AML debugger
2. Control method LID device
3. ACPICA release process
This patch also contains several trivial fixes detected during the documentation work.
Lv Zheng (5):
ACPI: Add documentation describing ACPICA release automation ACPI / debugger: Fix regressions that AML debugger stops working ACPI / debugger: Add AML debugger documentation
ACPI / button: Add SW_ACPI_LID for new usage model
ACPI: Add configuration item to configure ACPICA error logs out
Documentation/acpi/acpi-lid.txt | 42 ++++++ Documentation/acpi/aml-debugger.txt | 56 +++++++ Documentation/acpi/linuxized-acpica.txt | 251 +++++++++++++++++++++++++++++++ drivers/acpi/Kconfig | 7 + drivers/acpi/acpi_dbg.c | 6 +- drivers/acpi/button.c | 20 ++- include/acpi/platform/aclinux.h | 4 + include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input-event-codes.h | 3 +- 9 files changed, 381 insertions(+), 10 deletions(-) create mode 100644 Documentation/acpi/acpi-lid.txt create mode 100644 Documentation/acpi/aml-debugger.txt create mode 100644 Documentation/acpi/linuxized-acpica.txt
Signature · 1 line
1.7.10
PATCH 1/5 ACPI: Add documentation describing ACPICA release automation
This patch adds documentation on ACPICA release automation into the kernel Documentation/acpi folder.
--- Documentation/acpi/linuxized-acpica.txt | 251 +++++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 Documentation/acpi/linuxized-acpica.txt
Documentation/acpi/linuxized-acpica.txt new file +251 -0
@@ -0,0 +1,251 @@ +Linuxized ACPICA - Introduction of ACPICA Release Automation + +Copyright (C) 2013-2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +This document describes the ACPICA project and the relationship between +ACPICA and Linux. It also includes the descriptions on how ACPICA code is +automatically released to the following Linux subdirectories: +drivers/acpi/acpica, include/acpi and tools/power/acpi. + + +1. ACPICA Project + + The ACPI Component Architecture (ACPICA) project provides an operating + system (OS)-independent reference implementation of the Advanced + Configuration and Power Interface Specification (ACPI). It has been + adapted by various host OSes. By directly integrating ACPICA, Linux can + also benefit from the application experiences of ACPICA from other host + OSes. + + The homepage of ACPICA project is: www.acpica.org, it is maintained and + supported by Intel Corporation. + + The following figure dipicts the Linux ACPI subystem where the ACPICA + adaption is included: + + +---------------------------------------------------------+ + | | + | +---------------------------------------------------+ | + | | +------------------+ | | + | | | Table Management | | | + | | +------------------+ | | + | | +----------------------+ | | + | | | Namespace Management | | | + | | +----------------------+ | | + | | +------------------+ ACPICA Components | | + | | | Event Management | | | + | | +------------------+ | | + | | +---------------------+ | | + | | | Resource Management | | | + | | +---------------------+ | | + | | +---------------------+ | | + | | | Hardware Management | | | + | | +---------------------+ | | + | +---------------------------------------------------+ | | + | | | +------------------+ | | | + | | | | OS Service Layer | | | | + | | | +------------------+ | | | + | | +-------------------------------------------------|-+ | + | | +--------------------+ | | + | | | Device Enumeration | | | + | | +--------------------+ | | + | | +------------------+ | | + | | | Power Management | | | + | | +------------------+ Linux/ACPI Components | | + | | +--------------------+ | | + | | | Thermal Management | | | + | | +--------------------+ | | + | | +--------------------------+ | | + | | | Drivers for ACPI Devices | | | + | | +--------------------------+ | | + | | +--------+ | | + | | | ...... | | | + | | +--------+ | | + | +---------------------------------------------------+ | + | | + +---------------------------------------------------------+ + + Figure 1. Linux ACPI Software Components + + NOTE: + A. OS Service Layer - Provided by Linux to offer OS dependent + implementation of predefined ACPICA interfaces (acpi_os_*). + include/acpi/acpiosxf.h + drivers/acpi/osl.c + include/acpi/platform + B. ACPICA Functionalities - Released from ACPICA code base to offer + OS independent implementation of ACPICA interfaces (acpi_*). + drivers/acpi/acpica + include/acpi/ac*.h + tools/power/acpi + C. Linux/ACPI Functionalities - Providing Linux specific ACPI + functionalities to other Linux kernel subsystems and user space + programs. + drivers/acpi + include/linux/acpi.h + include/linux/acpi*.h + include/acpi + tools/power/acpi + D. Architecture Specific ACPICA/ACPI Functionalities - Provided by + ACPI subsystem to offer architecture specific implementation of ACPI + interfaces. They are Linux specific components, and is out of the + scope of this document. + include/asm/acpi.h + include/asm/acpi*.h + arch/*/acpi + +2. ACPICA Release + + ACPICA project maintains a code base at the following repository URL: + https://github.com/acpica/acpica.git. It is released once a month. + + As the coding style adopted by the ACPICA project is not acceptable by + Linux, there is a release process to convert the ACPICA GIT commits into + the Linux acceptable patches. The patches generated by this process are + known as "Linuxized ACPICA Patches". The release process is performed + against a local copy of the ACPICA git repository. Each commit in the + monthly release is converted into the linuxized ACPICA patch. They form + a montly ACPICA release patchset for Linux ACPI community. The + following figure dipicts the Linux upstream process of the ACPICA + commits: + + +-----------------------------+ + | acpica / master (-) commits | + +-----------------------------+ + /|\ | + | \|/ + | /---------------------\ +----------------------+ + | < Linuxize repo Utility >-->| old linuxized acpica |--+ + | \---------------------/ +----------------------+ | + | | + /---------\ | + < git reset > \ + \---------/ \ + /|\ /+-+ + | / | + +-----------------------------+ | | + | acpica / master (+) commits | | | + +-----------------------------+ | | + | | | + \|/ | | + /-----------------------\ +----------------------+ | | + < Linuxize repo Utilities >-->| new linuxized acpica |--+ | + \-----------------------/ +----------------------+ | + \|/ + +--------------------------+ /----------------------\ + | Linuxized ACPICA Patches |<----------------< Linuxize patch Utility > + +--------------------------+ \----------------------/ + | + \|/ + /---------------------------\ + < Linux ACPI Community Review > + \---------------------------/ + | + \|/ + +-----------------------+ /------------------\ +----------------+ + | linux-pm / linux-next |--< Linux Merge Window >-->| linux / master | + +-----------------------+ \------------------/ +----------------+ + + Figure 2. ACPICA -> Linux Upstream Process + + NOTE: + A. Linuxize Utilities - Provided by ACPICA repository, including a + utility located in source/tools/acpisrc folder and a bunch of + scripts located in generate/linux folder. + B. acpica / master - "master" branch of the git repository at + <https://github.com/acpica/acpica.git>. + C. linux-pm / linux-next - "linux-next" branch of the git repository at + <http://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git>. + D. linux / master - "master" branch of the git repository at + <http://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git>. + E. Linux ACPI Community - Reached at + <mailto:[email protected]>. + + Before the linuxized ACPICA patches are sent to the Linux ACPI community + for review, there is a quality ensurance build test process performed to + reduce porting issues. Currently this build process only cares about the + following kernel configurations: + CONFIG_ACPI/CONFIG_ACPI_DEBUG/CONFIG_ACPI_DEBUGGER + +3. ACPICA Divergence + + It's ideal that if all of the ACPICA commits are upstreamed + automatically without modifications, the "linux / master" tree should + contain the ACPICA code that is exactly same as the ACPICA code + contained in "new linuxized acpica" tree and the release process thus + could be performed automatically. + + But the facts are there are always source code differences between + Linux and ACPICA, such differences are known as "ACPICA Divergences". + + The various sources of the ACPICA divergences include: + 1. The existing divergences - Before the ACPICA release cycle is + fully running, there has already been divergences between Linux and + ACPICA. Over the past several years, the divergences are greatly + reduced, but there are still several ones and it takes time to figure + out the root cause of these old divergences. + 2. Manual modifications - Any manual modification (ex., coding style + fixes) done directly in the Linux side obviously hurts the ACPICA + release automation. Thus it is recommended to fix issues in the + ACPICA upstream and generate the linuxized fix using ACPICA release + utilities (please refer to the Chapter 4 for the details). + 3. Linux specific features - Sometime, it's impossible to use the + current ACPICA APIs to implement the features required by the Linux + kernel, the Linux developers have to hack ACPICA code directly. Such + hacks may not be acceptable from ACPICA's point of view, then they + are left as committed ACPICA divergences unless ACPICA side can + implement new mechanisms as replacements to cleanup these hacks. + 4. ACPICA release fixups - ACPICA only tests commits using a set of the + user space simulation utilies, thus the linuxized ACPICA patches may + break the Linux kernel, leaving us build/boot failures. In order not + to break the Linux bisection process, fixes are done directly to the + linuxized ACPICA patches during the release process. When the + release fixups are back ported to the ACPICA upstream, they must + follow the ACPICA upstream rules and thus risk further modifications. + These can all result in the new divergences. + 5. Quick path of ACPICA commits - Some ACPICA commits are good stable + materials, they are upstreamed prior than the ACPICA release process. + If such commits are reverted or rebased in ACPICA side in order to + offer better solutions, new ACPICA divergences are generated. + +4. ACPICA Development + + This paragraph guides the Linux developers to use the ACPICA upstream + release utilities to obtain the ACPICA upstream commits before they + are released by the ACPICA release process. + + 1. Cherry-pick an ACPICA commit: + First you need to git clone the ACPICA repository and the ACPICA changes + you want to cherry pick must be committed into the local repository. + Then the gen-patch.sh command can help to cherry-pick an ACPICA commit + from the ACPICA upstream. + # git clone https://github.com/acpica/acpica + # cd acpica + # generate/linux/gen-patch.sh -u [commit ID] + Here the commit ID is the ACPICA upstream commit ID you want to cherry + pick. It can be omitted if the commit is "HEAD". + + 2. Cherry-pick the recent ACPICA commits: + Sometimes you need to rebase your code on top of the recent ACPICA + changes that haven't been upstreamed to Linux yet. You can generate + the ACPICA release series on your own and rebase your code on top of + the generated ACPICA release patches. + # git clone https://github.com/acpica/acpica + # cd acpica + # generate/linux/make-patches.sh -u [commit ID] + The commit ID should be the last ACPICA commit accepted by Linux. + Normally it is a commit modifying ACPI_CA_VERSION. The modification can + be achieved by executing "git log source/include/acpixf.h" in the local + copy of the ACPICA repository. + + 3. Confirm the current divergences: + If you have local copies of both Linux and ACPICA, you can generate a + diff file indicating the state of the current divergences: + # git clone https://github.com/acpica/acpica + # git clone http://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git + # cd acpica + # generate/linux/divergences.sh -s ../linux
Signature · 1 line
1.7.10
PATCH 2/5 ACPI / debugger: Fix regressions that AML debugger stops working
In reply to Lv ZhengThe FIFO unlocking mechanism in acpi_dbg has been messed up by the following commit:
Commit: 287980e49ffc0f6d911601e7e352a812ed27768e
Subject: remove lots of IS_ERR_VALUE abuses
It converts !IS_ERR_VALUE(ret) into !ret. This patch fixes the regression.
--- drivers/acpi/acpi_dbg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/acpi/acpi_dbg.c +4 -2
@@ -602,7 +602,8 @@ static int acpi_aml_read_user(char __user *buf, int len) crc->tail = (crc->tail + n) & (ACPI_AML_BUF_SIZE -1 ); ret = n; out: -acpi_aml_unlock_fifo (ACPI_AML_OUT_USER , !ret); +acpi_aml_unlock_fifo (ACPI_AML_OUT_USER , + ret <0 ? false : true);return ret; }
@@ -672,7 +673,8 @@ static int acpi_aml_write_user(const char __user *buf, int len) crc->head = (crc->head + n) & (ACPI_AML_BUF_SIZE -1 ); ret = n; out: -acpi_aml_unlock_fifo (ACPI_AML_IN_USER , !ret); +acpi_aml_unlock_fifo (ACPI_AML_IN_USER , + ret <0 ? false : true);return n; }
Signature · 1 line
1.7.10
Rafael J. Wysocki
maintainerRe: [PATCH 2/5] ACPI / debugger: Fix regressions that AML debugger stops working
Quoted 10 lines · Lv Zheng
The FIFO unlocking mechanism in
acpi_dbghas been messed up by the following commit:
Commit:287980e49ffc0f6d911601e7e352a812ed27768e
Subject: remove lots ofIS_ERR_VALUEabuses
It converts !IS_ERR_VALUE(ret) into !ret. This patch fixes the regression.Fixes 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")Signed-off-by Lv Zheng <[email protected]>Cc Arnd Bergmann <[email protected]>
OK, but ->
Quoted 9 lines
--- drivers/acpi/acpi_dbg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/acpi/acpi_dbg.c +0
-0
drivers/acpi/acpi_dbg.c static int acpi_aml_read_user(char __user *buf, int len) · line 606
Quoted 6 lines
drivers/acpi/acpi_dbg.c +2
-1@@ -602,7 +602,8 @@ static int acpi_aml_read_user(char __user *buf, int len) crc->tail = (crc->tail + n) & (ACPI_AML_BUF_SIZE -1 ); ret = n; out: -acpi_aml_unlock_fifo (ACPI_AML_OUT_USER , !ret); +acpi_aml_unlock_fifo (ACPI_AML_OUT_USER , + ret <0 ? false : true);
-> The ternary operation is not necessary here, because the result of (ret < 0) is already boolean. So this can be written as
acpi_aml_unlock_fifo(ACPI_AML_OUT_USER, ret >= 0);
and analogously below.
I've made that change, please check the result in bleeding-edge.
Thanks,
Rafael
RE: [PATCH 2/5] ACPI / debugger: Fix regressions that AML debugger stops working
Hi, Rafael
Quoted 36 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J. Wysocki
Subject: Re: [PATCH 2/5] ACPI / debugger: Fix regressions that AML debugger stops workingQuoted 10 lines · Lv Zheng
The FIFO unlocking mechanism in
acpi_dbghas been messed up by the following commit:
Commit:287980e49ffc0f6d911601e7e352a812ed27768e
Subject: remove lots ofIS_ERR_VALUEabuses
It converts !IS_ERR_VALUE(ret) into !ret. This patch fixes the regression.Fixes 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")Signed-off-by Lv Zheng <[email protected]>Cc Arnd Bergmann <[email protected]>OK, but ->
drivers/acpi/acpi_dbg.c static int acpi_aml_read_user(char __user *buf,
Quoted 9 lines
--- drivers/acpi/acpi_dbg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/acpi/acpi_dbg.c +0
-0@@ -602,7 +602,8 @@ static int acpi_aml_read_user(char __user *buf,int len)
drivers/acpi/acpi_dbg.c static int acpi_aml_read_user(char __user *buf, int len) · line 606
Quoted 6 lines
drivers/acpi/acpi_dbg.c +2
-1crc->tail = (crc->tail + n) & (ACPI_AML_BUF_SIZE -1 ); ret = n; out: -acpi_aml_unlock_fifo (ACPI_AML_OUT_USER , !ret); +acpi_aml_unlock_fifo (ACPI_AML_OUT_USER , + ret <0 ? false : true);
Quoted 8 lines
-> The ternary operation is not necessary here, because the result of (ret < 0) is already boolean. So this can be written as
acpi_aml_unlock_fifo(ACPI_AML_OUT_USER, ret >= 0);and analogously below.
I've made that change, please check the result in bleeding-edge.
[Lv Zheng]
Confirmed to be working.
Thanks for the simplification.
Thanks and best regards
-Lv
PATCH 3/5 ACPI / debugger: Add AML debugger documentation
In reply to Lv ZhengThis patch adds AML debugger documentation.
--- Documentation/acpi/aml-debugger.txt | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 Documentation/acpi/aml-debugger.txt
Documentation/acpi/aml-debugger.txt new file +56 -0
@@ -0,0 +1,56 @@ +The AML Debugger + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +This document describes the usage of the AML debugger embedded in the Linux +kernel. + +1. Build the debugger + + The following kernel configuration items are required to enable the AML + debugger interface from the Linux kernel: + CONFIG_ACPI_DEBUGGER=y + CONFIG_ACPI_DEBUGGER_USER=m + The userspace utlities can be built from the kernel source tree using + the following commands: + # cd tools + # make acpi + The built userspace tool can be found at: + tools/acpi/power/acpi/acpidbg/acpidbg + You can install it to the system directories using the following + command: + # sudo make install + +2. Start the userspace debugger interface + + After booting the kernel with the debugger built-in, developers can + start the debugger with the following commands: + # sudo mount -t debugfs none /sys/kernel/debug + # sudo modprobe acpi_dbg + # sudo tools/acpi/power/acpi/acpidbg/acpidbg + Now you have entered the interactive AML debugger environment, where + you can execute the debugger functionalities by typing the debugger + commands. + You can start to use it from typing "help" command and can download + <ACPICA Overview and Programmer Reference> from the following site: + https://acpica.org/documentation + And find detailed command reference in "Chapter 12. ACPICA Debugger + Reference". + +3. Stop the userspace debugger interface + + You can type the Ctrl+C, quit, exit to end the interactive debugger + interface. And unload the module with the following commands: + # sudo rmmod acpi_dbg + The module unloading may fail if there is an acpidbg instance running. + +4. Run the debugger in a script + + It will be very useful to have the ability to run the AML debugger in + a test script. "acpidbg" supports this in a special "batch" mode. For + example, the following command outputs the entire ACPI namespace: + # sudo acpidbg -b "namespace"
Signature · 1 line
1.7.10
PATCH 4/5 ACPI / button: Add SW_ACPI_LID for new usage model
In reply to Lv ZhengThere are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
--- Documentation/acpi/acpi-lid.txt | 42 ++++++++++++++++++++++++++++++++ drivers/acpi/button.c | 20 ++++++++++----- include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input-event-codes.h | 3 ++- 4 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 Documentation/acpi/acpi-lid.txt
Documentation/acpi/acpi-lid.txt new file +42 -0
@@ -0,0 +1,42 @@ +Restrictions of ACPI Control Method LID Device + +1. Expections of _LID control method's returning value + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many BIOSen return the lid +state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is its initial +returning value. When the BIOSen implement this control method with cached +value, the initial returning value is likely not reliable. There are simply +so many examples always retuning "close" as initial lid state. + +2. Expections on lid events + +There are many BIOSen tables never notifying the lid open event. But it is +ensured that there is always lid close events reported when the lid is +closed. This is normally used to trigger system power saving operations on +Windows, thus it is fully tested and functions correctly. + +3. Linux ACPI Control Method LID Device Users + +The userspace should stop relying on /proc/acpi/button/lid/LID0/state to +obtain the lid state. This file is only used for the testing purpose. + +New userspace should rely on the lid close event to trigger power saving +operations and may stop taking actions according to the lid open event. A +new input event SW_ACPI_LID is prepared for the new userspace to implement +the ACPI control method lid device specific logics. + +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameter to +handle possible issues: + button.lid_init_state=method: + This is the default behavior, Linux kernel reports initial lid state + using _LID control method's returning value. + This may fixes some platforms if the _LID control method's returning + value is reliable. + button.lid_init_state=open: + Linux kernel always reports an initial lid state as "open". + This may fixes some platforms if the _LID control method's returning + value is not reliable.
drivers/acpi/button.c +14 -6
@@ -130,7 +130,8 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)return lid_state ?1 :0 ; } -static int acpi_lid_notify_state (struct acpi_device *device,int state) +static int acpi_lid_notify_state (struct acpi_device *device, +int state,bool notify_acpi) {struct acpi_button *button =acpi_driver_data (device);int ret;
@@ -138,6 +139,11 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)/* input layer checks if event is redundant */ input_report_switch (button->input,SW_LID , !state);input_sync (button->input); +if (notify_acpi) { +input_report_switch (button->input, +SW_ACPI_LID , !state); +input_sync (button->input); + }if (state)pm_wakeup_event (&device->dev,0 );
@@ -279,7 +285,8 @@ int acpi_lid_open(void) }EXPORT_SYMBOL (acpi_lid_open); -static int acpi_lid_update_state (struct acpi_device *device) +static int acpi_lid_update_state (struct acpi_device *device, +bool notify_acpi) {int state;
@@ -287,17 +294,17 @@ static int acpi_lid_update_state(struct acpi_device *device)if (state <0 )return state; -return acpi_lid_notify_state (device, state); +return acpi_lid_notify_state (device, state, notify_acpi); }static void acpi_lid_initialize_state (struct acpi_device *device) {switch (lid_init_state) {case ACPI_BUTTON_LID_INIT_OPEN : - (void )acpi_lid_notify_state (device,1 ); + (void )acpi_lid_notify_state (device,1 , false);break ;case ACPI_BUTTON_LID_INIT_METHOD : - (void )acpi_lid_update_state (device); + (void )acpi_lid_update_state (device, false);break ;case ACPI_BUTTON_LID_INIT_IGNORE :default :
@@ -317,7 +324,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)case ACPI_BUTTON_NOTIFY_STATUS : input = button->input;if (button->type ==ACPI_BUTTON_TYPE_LID ) { -acpi_lid_update_state (device); +acpi_lid_update_state (device, true); }else {int keycode;
@@ -436,6 +443,7 @@ static int acpi_button_add(struct acpi_device *device)case ACPI_BUTTON_TYPE_LID :input_set_capability (input,EV_SW ,SW_LID ); +input_set_capability (input,EV_SW ,SW_ACPI_LID );break ; }
include/linux/mod_devicetable.h +1 -1
@@ -291,7 +291,7 @@ struct pcmcia_device_id {#define INPUT_DEVICE_ID_LED_MAX 0x0f#define INPUT_DEVICE_ID_SND_MAX 0x07#define INPUT_DEVICE_ID_FF_MAX 0x7f -#define INPUT_DEVICE_ID_SW_MAX 0x0f +#define INPUT_DEVICE_ID_SW_MAX 0x10#define INPUT_DEVICE_ID_MATCH_BUS 1#define INPUT_DEVICE_ID_MATCH_VENDOR 2
include/uapi/linux/input-event-codes.h +2 -1
@@ -780,7 +780,8 @@#define SW_ROTATE_LOCK 0x0c/* set = rotate locked/disabled */ #define SW_LINEIN_INSERT 0x0d/* set = inserted */ #define SW_MUTE_DEVICE 0x0e/* set = device disabled */ -#define SW_MAX 0x0f +#define SW_ACPI_LID 0x0f/* set = lid shut */ +#define SW_MAX 0x10#define SW_CNT (SW_MAX+1)/*
Signature · 1 line
1.7.10
PATCH 5/5 ACPI: Add configuration item to configure ACPICA error logs out
In reply to Lv ZhengSometimes, we need to disable ACPICA error logs to leave only ACPICA debug logs enabled for debugging purpose. This is useful when ACPICA error logs become a flood.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=114201
--- drivers/acpi/Kconfig | 7 +++++++ include/acpi/platform/aclinux.h | 4 ++++ 2 files changed, 11 insertions(+)
drivers/acpi/Kconfig +7 -0
@@ -324,6 +324,13 @@ config ACPI_TABLE_UPGRADE initrd, therefore it's safe to say Y. See Documentation/acpi/initrd_table_override.txt for details +config ACPI_NO_ERROR_MESSAGES + bool "Disable error messages" + default n + help + The ACPI subsystem can produce error messages. Saying Y disables + this output. + config ACPI_DEBUG bool "Debug Statements" default n
include/acpi/platform/aclinux.h +4 -0
@@ -77,6 +77,10 @@#define ACPI_MUTEX_DEBUG #endif +#ifdef CONFIG_ACPI_NO_ERROR_MESSAGES +#define ACPI_NO_ERROR_MESSAGES +#endif +#include <linux/string.h> #include <linux/kernel.h> #include <linux/ctype.h>
Signature · 1 line
1.7.10
Rafael J. Wysocki
maintainerRe: [PATCH 5/5] ACPI: Add configuration item to configure ACPICA error logs out
Quoted 6 lines · Lv Zheng
Sometimes, we need to disable ACPICA error logs to leave only ACPICA debug logs enabled for debugging purpose. This is useful when ACPICA error logs become a flood.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=114201
Signed-off-by Lv Zheng <[email protected]>
I seem to remember seeing this change once before and ISTR I said I wouldn't apply it then.
Why would I say something different this time?
Quoted 39 lines
--- drivers/acpi/Kconfig | 7 +++++++ include/acpi/platform/aclinux.h | 4 ++++ 2 files changed, 11 insertions(+)drivers/acpi/Kconfig +7
-0@@ -324,6 +324,13 @@ config ACPI_TABLE_UPGRADE initrd, therefore it's safe to say Y. See Documentation/acpi/initrd_table_override.txt for details +config ACPI_NO_ERROR_MESSAGES + bool "Disable error messages" + default n + help + The ACPI subsystem can produce error messages. Saying Y disables + this output. + config ACPI_DEBUG bool "Debug Statements" default ninclude/acpi/platform/aclinux.h +4
-1@@ -77,6 +77,10 @@#define ACPI_MUTEX_DEBUG #endif +#ifdef CONFIG_ACPI_NO_ERROR_MESSAGES +#define ACPI_NO_ERROR_MESSAGES +#endif +#include <linux/string.h> #include <linux/kernel.h> #include <linux/ctype.h> --
RE: [PATCH 5/5] ACPI: Add configuration item to configure ACPICA error logs out
Hi, Rafael
Quoted 11 lines
From: [email protected] [mailto:linux-acpi- [email protected]] On Behalf Of Rafael J. Wysocki
Subject: Re: [PATCH 5/5] ACPI: Add configuration item to configure ACPICA error logs outQuoted 2 lines · Lv Zheng
Sometimes, we need to disable ACPICA error logs to leave only ACPICA debug logs enabled for debugging purpose. This is useful when ACPICA
error
Quoted 2 lines
logs become a flood.
Quoted 7 lines
Quoted 2 lines
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=114201
Signed-off-by Lv Zheng <[email protected]>I seem to remember seeing this change once before and ISTR I said I wouldn't apply it then.
Why would I say something different this time?
[Lv Zheng]
I missed that.
Now that I deleted this patch from my local queue.
Thanks and best regards
-Lv
Quoted 45 lines
Review
Quoted 25 lines
--- drivers/acpi/Kconfig | 7 +++++++ include/acpi/platform/aclinux.h | 4 ++++ 2 files changed, 11 insertions(+)drivers/acpi/Kconfig +7
-0@@ -324,6 +324,13 @@ config ACPI_TABLE_UPGRADE initrd, therefore it's safe to say Y. See Documentation/acpi/initrd_table_override.txt for details +config ACPI_NO_ERROR_MESSAGES + bool "Disable error messages" + default n + help + The ACPI subsystem can produce error messages. Saying Y disables + this output. + config ACPI_DEBUG bool "Debug Statements" default n(quoted diff) +0
-0
b/include/acpi/platform/aclinux.hQuoted 5 lines
index 93b61b1..ed27b52 100644
include/acpi/platform/aclinux.h +0
-0@@ -77,6 +77,10 @@#define ACPI_MUTEX_DEBUG include/acpi/platform/aclinux.h
Quoted 10 lines
include/acpi/platform/aclinux.h +4
-1#endif +#ifdef CONFIG_ACPI_NO_ERROR_MESSAGES +#define ACPI_NO_ERROR_MESSAGES +#endif +#include <linux/string.h> #include <linux/kernel.h> #include <linux/ctype.h> ----
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
v2 cover ACPI: ACPI documentation
In reply to Lv ZhengThis patch adds new ACPI documents related to:
1. ACPICA release process
2. AML debugger
3. ACPI control method lid device
This patch also contains an update of the ACPI LID input event in order to support the new usage model defined by the documentation change.
History:
1. Remove the upstreamed trivial fixes;
2. Split lid device code and documents;
3. Fix grammar issues in the documents.
Lv Zheng (4):
ACPI: Add documentation describing ACPICA release automation ACPI / debugger: Add AML debugger documentation
ACPI / button: Add SW_ACPI_LID for new usage model ACPI / button: Add document for ACPI control method lid device
restrictions
Documentation/acpi/acpi-lid.txt | 62 ++++++++ Documentation/acpi/aml-debugger.txt | 56 +++++++ Documentation/acpi/linuxized-acpica.txt | 253 +++++++++++++++++++++++++++++++ drivers/acpi/button.c | 20 ++- include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input-event-codes.h | 3 +- 6 files changed, 388 insertions(+), 8 deletions(-) create mode 100644 Documentation/acpi/acpi-lid.txt create mode 100644 Documentation/acpi/aml-debugger.txt create mode 100644 Documentation/acpi/linuxized-acpica.txt
Signature · 1 line
1.7.10
PATCH v2 1/4 ACPI: Add documentation describing ACPICA release automation
This patch adds documentation on ACPICA release automation into the kernel Documentation/acpi folder.
--- Documentation/acpi/linuxized-acpica.txt | 253 +++++++++++++++++++++++++++++++ 1 file changed, 253 insertions(+) create mode 100644 Documentation/acpi/linuxized-acpica.txt
Documentation/acpi/linuxized-acpica.txt new file +253 -0
@@ -0,0 +1,253 @@ +Linuxized ACPICA - Introduction of ACPICA Release Automation + +Copyright (C) 2013-2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +This document describes the ACPICA project and the relationship between +ACPICA and Linux. It also includes the descriptions on how ACPICA code is +automatically released to the following Linux subdirectories: +drivers/acpi/acpica, include/acpi and tools/power/acpi. + + +1. ACPICA Project + + The ACPI Component Architecture (ACPICA) project provides an operating + system (OS)-independent reference implementation of the Advanced + Configuration and Power Interface Specification (ACPI). It has been + adapted by various host OSes. By directly integrating ACPICA, Linux can + also benefit from the application experiences of ACPICA from other host + OSes. + + The homepage of ACPICA project is: www.acpica.org, it is maintained and + supported by Intel Corporation. + + The following figure dipicts the Linux ACPI subystem where the ACPICA + adaption is included: + + +---------------------------------------------------------+ + | | + | +---------------------------------------------------+ | + | | +------------------+ | | + | | | Table Management | | | + | | +------------------+ | | + | | +----------------------+ | | + | | | Namespace Management | | | + | | +----------------------+ | | + | | +------------------+ ACPICA Components | | + | | | Event Management | | | + | | +------------------+ | | + | | +---------------------+ | | + | | | Resource Management | | | + | | +---------------------+ | | + | | +---------------------+ | | + | | | Hardware Management | | | + | | +---------------------+ | | + | +---------------------------------------------------+ | | + | | | +------------------+ | | | + | | | | OS Service Layer | | | | + | | | +------------------+ | | | + | | +-------------------------------------------------|-+ | + | | +--------------------+ | | + | | | Device Enumeration | | | + | | +--------------------+ | | + | | +------------------+ | | + | | | Power Management | | | + | | +------------------+ Linux/ACPI Components | | + | | +--------------------+ | | + | | | Thermal Management | | | + | | +--------------------+ | | + | | +--------------------------+ | | + | | | Drivers for ACPI Devices | | | + | | +--------------------------+ | | + | | +--------+ | | + | | | ...... | | | + | | +--------+ | | + | +---------------------------------------------------+ | + | | + +---------------------------------------------------------+ + + Figure 1. Linux ACPI Software Components + + NOTE: + A. OS Service Layer - Provided by Linux to offer OS dependent + implementation of the predefined ACPICA interfaces (acpi_os_*). + include/acpi/acpiosxf.h + drivers/acpi/osl.c + include/acpi/platform + include/asm/acenv.h + B. ACPICA Functionalities - Released from ACPICA code base to offer + OS independent implementation of the ACPICA interfaces (acpi_*). + drivers/acpi/acpica + include/acpi/ac*.h + tools/power/acpi + C. Linux/ACPI Functionalities - Providing Linux specific ACPI + functionalities to the other Linux kernel subsystems and the user + space programs. + drivers/acpi + include/linux/acpi.h + include/linux/acpi*.h + include/acpi + tools/power/acpi + D. Architecture Specific ACPICA/ACPI Functionalities - Provided by the + ACPI subsystem to offer architecture specific implementation of the + ACPI interfaces. They are Linux specific components, and is out of + the scope of this document. + include/asm/acpi.h + include/asm/acpi*.h + arch/*/acpi + +2. ACPICA Release + + ACPICA project maintains a code base at the following repository URL: + https://github.com/acpica/acpica.git. It is released once a month. + + As the coding style adopted by the ACPICA project is not acceptable by + Linux, there is a release process to convert the ACPICA GIT commits into + the Linux acceptable patches. The patches generated by this process are + known as "Linuxized ACPICA Patches". The release process is performed + against a local copy of the ACPICA git repository. Each commit in the + monthly release is converted into the linuxized ACPICA patch. They form + a montly ACPICA release patchset for Linux ACPI community. The + following figure dipicts the Linux upstream process of the ACPICA + commits: + + +-----------------------------+ + | acpica / master (-) commits | + +-----------------------------+ + /|\ | + | \|/ + | /---------------------\ +----------------------+ + | < Linuxize repo Utility >-->| old linuxized acpica |--+ + | \---------------------/ +----------------------+ | + | | + /---------\ | + < git reset > \ + \---------/ \ + /|\ /+-+ + | / | + +-----------------------------+ | | + | acpica / master (+) commits | | | + +-----------------------------+ | | + | | | + \|/ | | + /-----------------------\ +----------------------+ | | + < Linuxize repo Utilities >-->| new linuxized acpica |--+ | + \-----------------------/ +----------------------+ | + \|/ + +--------------------------+ /----------------------\ + | Linuxized ACPICA Patches |<----------------< Linuxize patch Utility > + +--------------------------+ \----------------------/ + | + \|/ + /---------------------------\ + < Linux ACPI Community Review > + \---------------------------/ + | + \|/ + +-----------------------+ /------------------\ +----------------+ + | linux-pm / linux-next |--< Linux Merge Window >-->| linux / master | + +-----------------------+ \------------------/ +----------------+ + + Figure 2. ACPICA -> Linux Upstream Process + + NOTE: + A. Linuxize Utilities - Provided by the ACPICA repository, including a + utility located in source/tools/acpisrc folder and a bunch of + scripts located in generate/linux folder. + B. acpica / master - "master" branch of the git repository at + <https://github.com/acpica/acpica.git>. + C. linux-pm / linux-next - "linux-next" branch of the git repository at + <http://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git>. + D. linux / master - "master" branch of the git repository at + <http://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git>. + E. Linux ACPI Community - Reached at + <mailto:[email protected]>. + + Before the linuxized ACPICA patches are sent to the Linux ACPI community + for review, there is a quality ensurance build test process performed to + reduce porting issues. Currently this build process only cares about + the following kernel configurations: + CONFIG_ACPI/CONFIG_ACPI_DEBUG/CONFIG_ACPI_DEBUGGER + +3. ACPICA Divergence + + It's ideal that if all of the ACPICA commits are upstreamed + automatically without modifications, the "linux / master" tree should + contain the ACPICA code that is exactly same as the ACPICA code + contained in "new linuxized acpica" tree and the release process thus + could be performed automatically. + + But the facts are there are always source code differences between + Linux and ACPICA, such differences are known as "ACPICA Divergences". + + The various sources of the ACPICA divergences include: + 1. The existing divergences - Before the ACPICA release cycle is + fully running, there has already been divergences between Linux and + ACPICA. Over the past several years, the divergences are greatly + reduced, but there are still several ones and it takes time to figure + out the root cause of these old divergences. + 2. Manual modifications - Any manual modification (ex., coding style + fixes) done directly in the Linux side obviously hurts the ACPICA + release automation. Thus it is recommended to fix such issues in the + ACPICA upstream and generate the linuxized fix using the ACPICA + release utilities (please refer to the Chapter 4 for the details). + 3. Linux specific features - Sometime, it's impossible to use the + current ACPICA APIs to implement the features required by the Linux + kernel, the Linux developers have to hack ACPICA code directly. Such + hacks may not be acceptable from ACPICA's point of view, then they + are left as committed ACPICA divergences unless ACPICA side can + implement new mechanisms as replacements to clean these hacks up. + 4. ACPICA release fixups - ACPICA only tests commits using a set of the + user space simulation utilies, thus the linuxized ACPICA patches may + break the Linux kernel, leaving us build/boot failures. In order not + to break the Linux bisection process, fixes are done directly to the + linuxized ACPICA patches during the release process. When the + release fixups are back ported to the ACPICA upstream, they must + follow the ACPICA upstream rules and thus risk further modifications. + These can all result in the new divergences. + 5. Quick path of the ACPICA commits - Some ACPICA commits are good + stable materials, they are upstreamed prior than the ACPICA release + process. If such commits are reverted or rebased in the ACPICA side + in order to offer better solutions, new ACPICA divergences are + generated. + +4. ACPICA Development + + This paragraph guides the Linux developers to use the ACPICA upstream + release utilities to obtain the ACPICA upstream commits before they + are released by the ACPICA release process. + + 1. Cherry-pick an ACPICA commit: + First you need to git clone the ACPICA repository and the ACPICA changes + you want to cherry pick must be committed into the local repository. + Then the gen-patch.sh command can help to cherry-pick an ACPICA commit + from the ACPICA local repository. + # git clone https://github.com/acpica/acpica + # cd acpica + # generate/linux/gen-patch.sh -u [commit ID] + Here the commit ID is the ACPICA local repository commit ID you want to + cherry pick. It can be omitted if the commit is "HEAD". + + 2. Cherry-pick the recent ACPICA commits: + Sometimes you need to rebase your code on top of the recent ACPICA + changes that haven't been upstreamed to Linux yet. You can generate + the ACPICA release series on your own and rebase your code on top of + the generated ACPICA release patches. + # git clone https://github.com/acpica/acpica + # cd acpica + # generate/linux/make-patches.sh -u [commit ID] + The commit ID should be the last ACPICA commit accepted by Linux. + Normally it is a commit modifying ACPI_CA_VERSION. The modification can + be achieved by executing "git blame source/include/acpixf.h" and + referencing line that contains "ACPI_CA_VERSION". + + 3. Confirm the current divergences: + If you have local copies of both Linux and ACPICA, you can generate a + diff file indicating the state of the current divergences: + # git clone https://github.com/acpica/acpica + # git clone http://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git + # cd acpica + # generate/linux/divergences.sh -s ../linux
Signature · 1 line
1.7.10
PATCH v2 2/4 ACPI / debugger: Add AML debugger documentation
In reply to Lv ZhengThis patch adds AML debugger documentation.
--- Documentation/acpi/aml-debugger.txt | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 Documentation/acpi/aml-debugger.txt
Documentation/acpi/aml-debugger.txt new file +56 -0
@@ -0,0 +1,56 @@ +The AML Debugger + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +This document describes the usage of the AML debugger embedded in the Linux +kernel. + +1. Build the debugger + + The following kernel configuration items are required to enable the AML + debugger interface from the Linux kernel: + CONFIG_ACPI_DEBUGGER=y + CONFIG_ACPI_DEBUGGER_USER=m + The userspace utlities can be built from the kernel source tree using + the following commands: + # cd tools + # make acpi + The built userspace tool can be found at: + tools/acpi/power/acpi/acpidbg/acpidbg + You can install it to the system directories using the following + command: + # make install + +2. Start the userspace debugger interface + + After booting the kernel with the debugger built-in, developers can + start the debugger with the following commands: + # mount -t debugfs none /sys/kernel/debug + # modprobe acpi_dbg + # tools/acpi/power/acpi/acpidbg/acpidbg + Now you have entered the interactive AML debugger environment, where + you can execute the debugger functionalities by typing the debugger + commands. + You can start to use it from typing "help" command and can download + <ACPICA Overview and Programmer Reference> from the following site: + https://acpica.org/documentation + And find detailed command reference in "Chapter 12. ACPICA Debugger + Reference". + +3. Stop the userspace debugger interface + + You can type the Ctrl+C, quit, exit to end the interactive debugger + interface. And unload the module with the following commands: + # rmmod acpi_dbg + The module unloading may fail if there is an acpidbg instance running. + +4. Run the debugger in a script + + It will be very useful to have the ability to run the AML debugger in + a test script. "acpidbg" supports this in a special "batch" mode. For + example, the following command outputs the entire ACPI namespace: + # acpidbg -b "namespace"
Signature · 1 line
1.7.10
PATCH v2 3/4 ACPI / button: Add SW_ACPI_LID for new usage model
In reply to Lv ZhengThere are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds a new input key event so that new userspace programs can use it to handle this usage model correctly. And in the meanwhile, no old programs will be broken by the userspace changes.
--- drivers/acpi/button.c | 20 ++++++++++++++------ include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input-event-codes.h | 3 ++- 3 files changed, 17 insertions(+), 8 deletions(-)
drivers/acpi/button.c +14 -6
@@ -130,7 +130,8 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)return lid_state ?1 :0 ; } -static int acpi_lid_notify_state (struct acpi_device *device,int state) +static int acpi_lid_notify_state (struct acpi_device *device, +int state,bool notify_acpi) {struct acpi_button *button =acpi_driver_data (device);int ret;
@@ -138,6 +139,11 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)/* input layer checks if event is redundant */ input_report_switch (button->input,SW_LID , !state);input_sync (button->input); +if (notify_acpi) { +input_report_switch (button->input, +SW_ACPI_LID , !state); +input_sync (button->input); + }if (state)pm_wakeup_event (&device->dev,0 );
@@ -279,7 +285,8 @@ int acpi_lid_open(void) }EXPORT_SYMBOL (acpi_lid_open); -static int acpi_lid_update_state (struct acpi_device *device) +static int acpi_lid_update_state (struct acpi_device *device, +bool notify_acpi) {int state;
@@ -287,17 +294,17 @@ static int acpi_lid_update_state(struct acpi_device *device)if (state <0 )return state; -return acpi_lid_notify_state (device, state); +return acpi_lid_notify_state (device, state, notify_acpi); }static void acpi_lid_initialize_state (struct acpi_device *device) {switch (lid_init_state) {case ACPI_BUTTON_LID_INIT_OPEN : - (void )acpi_lid_notify_state (device,1 ); + (void )acpi_lid_notify_state (device,1 , false);break ;case ACPI_BUTTON_LID_INIT_METHOD : - (void )acpi_lid_update_state (device); + (void )acpi_lid_update_state (device, false);break ;case ACPI_BUTTON_LID_INIT_IGNORE :default :
@@ -317,7 +324,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)case ACPI_BUTTON_NOTIFY_STATUS : input = button->input;if (button->type ==ACPI_BUTTON_TYPE_LID ) { -acpi_lid_update_state (device); +acpi_lid_update_state (device, true); }else {int keycode;
@@ -436,6 +443,7 @@ static int acpi_button_add(struct acpi_device *device)case ACPI_BUTTON_TYPE_LID :input_set_capability (input,EV_SW ,SW_LID ); +input_set_capability (input,EV_SW ,SW_ACPI_LID );break ; }
include/linux/mod_devicetable.h +1 -1
@@ -291,7 +291,7 @@ struct pcmcia_device_id {#define INPUT_DEVICE_ID_LED_MAX 0x0f#define INPUT_DEVICE_ID_SND_MAX 0x07#define INPUT_DEVICE_ID_FF_MAX 0x7f -#define INPUT_DEVICE_ID_SW_MAX 0x0f +#define INPUT_DEVICE_ID_SW_MAX 0x10#define INPUT_DEVICE_ID_MATCH_BUS 1#define INPUT_DEVICE_ID_MATCH_VENDOR 2
include/uapi/linux/input-event-codes.h +2 -1
@@ -780,7 +780,8 @@#define SW_ROTATE_LOCK 0x0c/* set = rotate locked/disabled */ #define SW_LINEIN_INSERT 0x0d/* set = inserted */ #define SW_MUTE_DEVICE 0x0e/* set = device disabled */ -#define SW_MAX 0x0f +#define SW_ACPI_LID 0x0f/* set = lid shut */ +#define SW_MAX 0x10#define SW_CNT (SW_MAX+1)/*
Signature · 1 line
1.7.10
Re: [PATCH v2 3/4] ACPI / button: Add SW_ACPI_LID for new usage model
Quoted 39 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds a new input key event so that new userspace programs can use it to handle this usage model correctly. And in the meanwhile, no old programs will be broken by the userspace changes.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- drivers/acpi/button.c | 20 ++++++++++++++------ include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input-event-codes.h | 3 ++- 3 files changed, 17 insertions(+), 8 deletions(-)drivers/acpi/button.c +2
-1@@ -130,7 +130,8 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)return lid_state ?1 :0 ; } -static int acpi_lid_notify_state (struct acpi_device *device,int state) +static int acpi_lid_notify_state (struct acpi_device *device, +int state,bool notify_acpi) {struct acpi_button *button =acpi_driver_data (device);int ret;
drivers/acpi/button.c static int acpi_lid_notify_state(struct acpi_device *device, int state) · line 145
Quoted 7 lines
drivers/acpi/button.c +4
-0@@ -138,6 +139,11 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)/* input layer checks if event is redundant */ input_report_switch (button->input,SW_LID , !state);input_sync (button->input); +if (notify_acpi) { +input_report_switch (button->input, +SW_ACPI_LID , !state); +input_sync (button->input);
If you use a switch, you'll never send subsequent open state if you doesn't close it yourself.
See my comments in 5/5 and please use a KEY event instead.
Quoted 45 lines
drivers/acpi/button.c +7
-5+ }if (state)pm_wakeup_event (&device->dev,0 );@@ -279,7 +285,8 @@ int acpi_lid_open(void) }EXPORT_SYMBOL (acpi_lid_open); -static int acpi_lid_update_state (struct acpi_device *device) +static int acpi_lid_update_state (struct acpi_device *device, +bool notify_acpi) {int state;@@ -287,17 +294,17 @@ static int acpi_lid_update_state(struct acpi_device *device)if (state <0 )return state; -return acpi_lid_notify_state (device, state); +return acpi_lid_notify_state (device, state, notify_acpi); }static void acpi_lid_initialize_state (struct acpi_device *device) {switch (lid_init_state) {case ACPI_BUTTON_LID_INIT_OPEN : - (void )acpi_lid_notify_state (device,1 ); + (void )acpi_lid_notify_state (device,1 , false);break ;case ACPI_BUTTON_LID_INIT_METHOD : - (void )acpi_lid_update_state (device); + (void )acpi_lid_update_state (device, false);break ;case ACPI_BUTTON_LID_INIT_IGNORE :default :@@ -317,7 +324,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)case ACPI_BUTTON_NOTIFY_STATUS : input = button->input;if (button->type ==ACPI_BUTTON_TYPE_LID ) { -acpi_lid_update_state (device); +acpi_lid_update_state (device, true); }else {int keycode;
drivers/acpi/button.c static int acpi_button_add(struct acpi_device *device) · line 446
Quoted 4 lines
drivers/acpi/button.c +1
-0@@ -436,6 +443,7 @@ static int acpi_button_add(struct acpi_device *device)case ACPI_BUTTON_TYPE_LID :input_set_capability (input,EV_SW ,SW_LID ); +input_set_capability (input,EV_SW ,SW_ACPI_LID );
Can't we export this new event only if the _LID function is not reliable? This could check for the module parameter lid_init_state and only enable it for ACPI_BUTTON_LID_INIT_OPEN.
I really hope we will be able to find a reliable way to determine whether or not the platform support reliable LID state. If not, there might be a need to have a db of reliable switch platforms. This can be set in the kernel or with a hwdb entry in userspace.
Cheers,
Benjamin
Quoted 33 lines
drivers/acpi/button.c +0
-0break ; }include/linux/mod_devicetable.h +1
-1@@ -291,7 +291,7 @@ struct pcmcia_device_id {#define INPUT_DEVICE_ID_LED_MAX 0x0f#define INPUT_DEVICE_ID_SND_MAX 0x07#define INPUT_DEVICE_ID_FF_MAX 0x7f -#define INPUT_DEVICE_ID_SW_MAX 0x0f +#define INPUT_DEVICE_ID_SW_MAX 0x10#define INPUT_DEVICE_ID_MATCH_BUS 1#define INPUT_DEVICE_ID_MATCH_VENDOR 2include/uapi/linux/input-event-codes.h +2
-2@@ -780,7 +780,8 @@#define SW_ROTATE_LOCK 0x0c/* set = rotate locked/disabled */ #define SW_LINEIN_INSERT 0x0d/* set = inserted */ #define SW_MUTE_DEVICE 0x0e/* set = device disabled */ -#define SW_MAX 0x0f +#define SW_ACPI_LID 0x0f/* set = lid shut */ +#define SW_MAX 0x10#define SW_CNT (SW_MAX+1)/* -- 1.7.10
Dmitry Torokhov
maintainerRe: [PATCH v2 3/4] ACPI / button: Add SW_ACPI_LID for new usage model
Quoted 13 lines · Benjamin Tissoires
Quoted 12 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds a new input key event so that new userspace programs can use it to handle this usage model correctly. And in the meanwhile, no old programs will be broken by the userspace changes.
Quoted 1 line
Quoted 1 line
Does not work for me.
Quoted 1 line
Quoted 1 line
Gives me info about pull "Basic DNSSEC support, and unrelated fixes"
Quoted 113 lines
Quoted 25 lines
Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- drivers/acpi/button.c | 20 ++++++++++++++------ include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input-event-codes.h | 3 ++- 3 files changed, 17 insertions(+), 8 deletions(-)drivers/acpi/button.c +2
-1@@ -130,7 +130,8 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)return lid_state ?1 :0 ; } -static int acpi_lid_notify_state (struct acpi_device *device,int state) +static int acpi_lid_notify_state (struct acpi_device *device, +int state,bool notify_acpi) {struct acpi_button *button =acpi_driver_data (device);int ret;drivers/acpi/button.c static int acpi_lid_notify_state(struct acpi_device *device, int state) · line 145
Quoted 7 lines
drivers/acpi/button.c +4
-0@@ -138,6 +139,11 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)/* input layer checks if event is redundant */ input_report_switch (button->input,SW_LID , !state);input_sync (button->input); +if (notify_acpi) { +input_report_switch (button->input, +SW_ACPI_LID , !state); +input_sync (button->input);If you use a switch, you'll never send subsequent open state if you doesn't close it yourself.
See my comments in 5/5 and please use a KEY event instead.Quoted 45 lines
drivers/acpi/button.c +7
-5+ }if (state)pm_wakeup_event (&device->dev,0 );@@ -279,7 +285,8 @@ int acpi_lid_open(void) }EXPORT_SYMBOL (acpi_lid_open); -static int acpi_lid_update_state (struct acpi_device *device) +static int acpi_lid_update_state (struct acpi_device *device, +bool notify_acpi) {int state;@@ -287,17 +294,17 @@ static int acpi_lid_update_state(struct acpi_device *device)if (state <0 )return state; -return acpi_lid_notify_state (device, state); +return acpi_lid_notify_state (device, state, notify_acpi); }static void acpi_lid_initialize_state (struct acpi_device *device) {switch (lid_init_state) {case ACPI_BUTTON_LID_INIT_OPEN : - (void )acpi_lid_notify_state (device,1 ); + (void )acpi_lid_notify_state (device,1 , false);break ;case ACPI_BUTTON_LID_INIT_METHOD : - (void )acpi_lid_update_state (device); + (void )acpi_lid_update_state (device, false);break ;case ACPI_BUTTON_LID_INIT_IGNORE :default :@@ -317,7 +324,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)case ACPI_BUTTON_NOTIFY_STATUS : input = button->input;if (button->type ==ACPI_BUTTON_TYPE_LID ) { -acpi_lid_update_state (device); +acpi_lid_update_state (device, true); }else {int keycode;drivers/acpi/button.c static int acpi_button_add(struct acpi_device *device) · line 446
Quoted 4 lines
drivers/acpi/button.c +1
-0@@ -436,6 +443,7 @@ static int acpi_button_add(struct acpi_device *device)case ACPI_BUTTON_TYPE_LID :input_set_capability (input,EV_SW ,SW_LID ); +input_set_capability (input,EV_SW ,SW_ACPI_LID );Can't we export this new event only if the
_LIDfunction is not reliable? This could check for the module parameterlid_init_stateand only enable it forACPI_BUTTON_LID_INIT_OPEN.I really hope we will be able to find a reliable way to determine whether or not the platform support reliable LID state. If not, there might be a need to have a db of reliable switch platforms. This can be set in the kernel or with a hwdb entry in userspace.
Cheers,
BenjaminQuoted 14 lines
drivers/acpi/button.c +0
-0break ; }include/linux/mod_devicetable.h +1
-1@@ -291,7 +291,7 @@ struct pcmcia_device_id {#define INPUT_DEVICE_ID_LED_MAX 0x0f#define INPUT_DEVICE_ID_SND_MAX 0x07#define INPUT_DEVICE_ID_FF_MAX 0x7f -#define INPUT_DEVICE_ID_SW_MAX 0x0f +#define INPUT_DEVICE_ID_SW_MAX 0x10
Quoted 12 lines
include/uapi/linux/input-event-codes.h line 783
Quoted 12 lines
include/linux/mod_devicetable.h +0
-0#define INPUT_DEVICE_ID_MATCH_BUS 1#define INPUT_DEVICE_ID_MATCH_VENDOR 2include/uapi/linux/input-event-codes.h +1
-1@@ -780,7 +780,8 @@#define SW_ROTATE_LOCK 0x0c/* set = rotate locked/disabled */ #define SW_LINEIN_INSERT 0x0d/* set = inserted */ #define SW_MUTE_DEVICE 0x0e/* set = device disabled */ -#define SW_MAX 0x0f +#define SW_ACPI_LID 0x0f/* set = lid shut */
0x0f is busy already.
Quoted 7 lines
Quoted 7 lines
include/uapi/linux/input-event-codes.h +1
-1+#define SW_MAX 0x10#define SW_CNT (SW_MAX+1)/* -- 1.7.10
Signature · 1 line
Dmitry
PATCH v2 4/4 ACPI / button: Add document for ACPI control method lid device restrictions
In reply to Lv ZhengThere are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.
--- Documentation/acpi/acpi-lid.txt | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt
Documentation/acpi/acpi-lid.txt new file +62 -0
@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs using a +control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must be implemented to +report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of the Linux +ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables return the lid +state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is its initial +returning value. When the AML tables implement this control method with +cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state is +changed to "opened". But it is ensured that the AML tables always notify +"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose. + +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actions according +to the lid "opened" notification. A new input switch event - SW_ACPI_LID is +prepared for the new userspace to implement this ACPI control method lid +device specific logics. + +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameter to +handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.
Signature · 1 line
1.7.10
Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Hi,
Quoted 59 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txtDocumentation/acpi/acpi-lid.txt new file +32
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs using a +control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must be implemented to +report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of the Linux +ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables return the lid +state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is its initial +returning value. When the AML tables implement this control method with +cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state is
Documentation/acpi/acpi-lid.txt line 42
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+changed to "opened". But it is ensured that the AML tables always notify +"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose.
I'd say: this file actually calls the _LID method described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.
Documentation/acpi/acpi-lid.txt line 48
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actions according +to the lid "opened" notification. A new input switch event - SW_ACPI_LID is +prepared for the new userspace to implement this ACPI control method lid +device specific logics.
That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.
Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Please use a simple KEY_LID_OPEN event you will send through [input_key_event(KEY_LID_OPEN, 1), input_sync(), input_key_event(KEY_LID_OPEN, 0), input_sync()], which userspace knows how to handle.
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameter to +handle possible issues:
Documentation/acpi/acpi-lid.txt line 62
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+ button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.
This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending SW_LID for reliable LID platforms, and not remove it entirely as you will break platforms.
Cheers,
Benjamin
Quoted 3 lines
--
1.7.10
Dmitry Torokhov
maintainerRe: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Quoted 123 lines · Benjamin Tissoires
Hi,
Quoted 59 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txtDocumentation/acpi/acpi-lid.txt new file +32
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs using a +control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must be implemented to +report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of the Linux +ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables return the lid +state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is its initial +returning value. When the AML tables implement this control method with +cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state isDocumentation/acpi/acpi-lid.txt line 42
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+changed to "opened". But it is ensured that the AML tables always notify +"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 48
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actions according +to the lid "opened" notification. A new input switch event - SW_ACPI_LID is +prepared for the new userspace to implement this ACPI control method lid +device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspace knows how to handle.Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameter to +handle possible issues:Documentation/acpi/acpi-lid.txt line 62
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+ button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
Quoted 2 lines
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.
How about we leave the kernel alone and userspace (which would have to cope with the new KEY_LID_OPEN anyway) would just have to know that if switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0) then it can't trust the events and it needs additional heuristics.
Thanks.
Signature · 1 line
Dmitry
Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Quoted 124 lines · Dmitry Torokhov
Quoted 123 lines · Benjamin Tissoires
Hi,
Quoted 59 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txtDocumentation/acpi/acpi-lid.txt new file +32
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs using a +control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must be implemented to +report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of the Linux +ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables return the lid +state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is its initial +returning value. When the AML tables implement this control method with +cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state isDocumentation/acpi/acpi-lid.txt line 42
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+changed to "opened". But it is ensured that the AML tables always notify +"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 48
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actions according +to the lid "opened" notification. A new input switch event - SW_ACPI_LID is +prepared for the new userspace to implement this ACPI control method lid +device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspace knows how to handle.Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameter to +handle possible issues:Documentation/acpi/acpi-lid.txt line 62
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+ button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
Quoted 8 lines
Quoted 2 lines
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have to cope with the new
KEY_LID_OPENanyway) would just have to know that if switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0) then it can't trust the events and it needs additional heuristics.
I really wished we could leave the kernel alone, but some platform need fixes: we are using an EV_SW, and on those platform, we only get the close event, which means it gets ignored by the input layer. Those platform have a form factor which makes the situation acceptable (tablet with cover, or very low cost laptops).
However, switching away from a different EV_SW and tell userspace to ignore it would break systems when you are using a docking station. The acpi would provide fake values for the LID switch and this will screw the session if you are working on a docking station with an external monitor and the LID closed.
My initial suggestion was:
- detect in the kernel whether the ACPI LID information was judged as reliable
- if reliable keep things as it
- if not reliable add an extra KEY_LID_CLOSE to notify userspace that the SW_LID is not reliable (it will emulate the LID open events) and that it will only get true close events.
After further thoughts, I think we can simply add the extra key, and have an hwdb entry in logind to enumerate the devices only relying on the close event. If the userspace is not updated, we can tell user to use the button.lid_init_state=open quirk to simulate the behavior logind should expect.
That means only adding one extra key (and its events) in the kernel and let userspace decide what to do.
Cheers,
Benjamin
Dmitry Torokhov
maintainerRe: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Quoted 135 lines · Benjamin Tissoires
Quoted 124 lines · Dmitry Torokhov
Quoted 123 lines · Benjamin Tissoires
Hi,
Quoted 59 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txtDocumentation/acpi/acpi-lid.txt new file +32
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs using a +control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must be implemented to +report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of the Linux +ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables return the lid +state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is its initial +returning value. When the AML tables implement this control method with +cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state isDocumentation/acpi/acpi-lid.txt line 42
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+changed to "opened". But it is ensured that the AML tables always notify +"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 48
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actions according +to the lid "opened" notification. A new input switch event - SW_ACPI_LID is +prepared for the new userspace to implement this ACPI control method lid +device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspace knows how to handle.Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameter to +handle possible issues:Documentation/acpi/acpi-lid.txt line 62
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+ button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
Quoted 8 lines
Quoted 2 lines
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have to cope with the new
KEY_LID_OPENanyway) would just have to know that if switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0) then it can't trust the events and it needs additional heuristics.
Quoted 3 lines
I really wished we could leave the kernel alone, but some platform need fixes: we are using an
EV_SW, and on those platform, we only get the close event, which means it gets ignored by the input layer.
OK. Can we then emit missing "open" if we get "close" and the state is already closed?
Thanks.
Signature · 1 line
Dmitry
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Hi, Dmitry
Quoted 175 lines
From: Dmitry Torokhov [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 6 lines · Benjamin Tissoires
Quoted 4 lines · Dmitry Torokhov
Quoted 3 lines · Benjamin Tissoires
Hi,
On Thu, Jul 7, 2016 at 9:11 AM, Lv Zheng <[email protected]>
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
There are many AML tables reporting wrong initial lid state, and
some of
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event whichis
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0
b/Documentation/acpi/acpi-lid.txtQuoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt new file +1
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid DeviceQuoted 8 lines
Quoted 8 lines
Quoted 8 lines
Documentation/acpi/acpi-lid.txt
Quoted 8 lines
Documentation/acpi/acpi-lid.txt new file +8
-0+ +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing a
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lidstate has
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lidstate.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 22
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+However the word of "current" has ambiguity, many AML tablesreturn the lid
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this controlmethod with
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 27
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+cached value, the initial returning value is likely not reliable. Thereare
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 28
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+simply so many examples always retuning "closed" as initial lidstate.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid devicestate is
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 34
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 35
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows.Since it is
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 44
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed"notification to
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod lid
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
how to handle.
Documentation/acpi/acpi-lid.txt line 50
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +During the period the userspace hasn't been switched to use thenew
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 51
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+SW_ACPI_LID event, Linux users can use the following bootparameter to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linuxkernel
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID controlmethod's
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 9 lines
Quoted 9 lines
Quoted 7 lines
Documentation/acpi/acpi-lid.txt line 62
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have
to
Quoted 1 line
Quoted 1 line
cope with the new
KEY_LID_OPENanyway) would just have to knowthat if
Quoted 1 line
Quoted 1 line
switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0)
then it
Quoted 3 lines
Quoted 2 lines
can't trust the events and it needs additional heuristics.
Quoted 6 lines
Quoted 3 lines
I really wished we could leave the kernel alone, but some platform need fixes: we are using an
EV_SW, and on those platform, we only get the close event, which means it gets ignored by the input layer.OK. Can we then emit missing "open" if we get "close" and the state is already closed?
[Lv Zheng]
The problem is systemd/logind thinks SW_LID is a switch event, thus it must be paired. While the real world is: there are platforms, SW_LID may not be paired for ACPI control method lid device.
As the events are provided by the BIOS tables, the Linux ACPI subsystem actually has no idea on how to get the missing "open" back. Unless we hire people to wait for the bug reports and fix the BIOS tables.
Thanks and best regards
-Lv
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
In reply to Dmitry TorokhovHi, Dmitry
Quoted 175 lines
From: Dmitry Torokhov [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 6 lines · Benjamin Tissoires
Quoted 4 lines · Dmitry Torokhov
Quoted 3 lines · Benjamin Tissoires
Hi,
On Thu, Jul 7, 2016 at 9:11 AM, Lv Zheng <[email protected]>
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
There are many AML tables reporting wrong initial lid state, and
some of
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event whichis
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0
b/Documentation/acpi/acpi-lid.txtQuoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt new file +1
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid DeviceQuoted 8 lines
Quoted 8 lines
Quoted 8 lines
Documentation/acpi/acpi-lid.txt
Quoted 8 lines
Documentation/acpi/acpi-lid.txt new file +8
-0+ +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing a
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lidstate has
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lidstate.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 22
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+However the word of "current" has ambiguity, many AML tablesreturn the lid
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this controlmethod with
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 27
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+cached value, the initial returning value is likely not reliable. Thereare
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 28
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+simply so many examples always retuning "closed" as initial lidstate.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid devicestate is
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 34
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 35
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows.Since it is
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 44
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed"notification to
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod lid
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
how to handle.
Documentation/acpi/acpi-lid.txt line 50
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +During the period the userspace hasn't been switched to use thenew
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 51
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+SW_ACPI_LID event, Linux users can use the following bootparameter to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linuxkernel
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID controlmethod's
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 9 lines
Quoted 9 lines
Quoted 7 lines
Documentation/acpi/acpi-lid.txt line 62
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have
to
Quoted 1 line
Quoted 1 line
cope with the new
KEY_LID_OPENanyway) would just have to knowthat if
Quoted 1 line
Quoted 1 line
switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0)
then it
Quoted 3 lines
Quoted 2 lines
can't trust the events and it needs additional heuristics.
Quoted 6 lines
Quoted 3 lines
I really wished we could leave the kernel alone, but some platform need fixes: we are using an
EV_SW, and on those platform, we only get the close event, which means it gets ignored by the input layer.OK. Can we then emit missing "open" if we get "close" and the state is already closed?
[Lv Zheng]
I've been considering this again.
I think you may mean we can improve SW_LID by reporting SW_LID(open) when a new close event is arrived. So that we may be able to fix old programs.
Possibly the code should be looked like:
If (!!state != last_state && time_after(jiffies, last_jiffies)) {
input_report_switch(..., state);
}input_report_switch(..., !state);
However, there are tables never reporting "open".
If we do things in this way, there will be a very long period between the last close and the next close.
Which means there will be a too long period between the close and the open switch events. IMO, this cannot fix old user space programs.
Currently, logind has a timeout mechanism.
If it cannot receive "open" within this period, it will suspend the system again right after resuming. Thus we can still see that suspending right after resuming could be resulted.
However the new key events allow the new user space to fix the issue without changing the timeout logic.
For the old user space programs, I have no idea how we can fix them on such platforms (no open events).
Thanks and best regards
-Lv
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
In reply to Benjamin TissoiresHi,
Quoted 180 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 5 lines · Dmitry Torokhov
Quoted 4 lines · Benjamin Tissoires
Hi,
Quoted 1 line · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some
of
Quoted 1 line
Quoted 1 line
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Quoted 4 lines
Quoted 4 lines
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0
b/Documentation/acpi/acpi-lid.txtQuoted 6 lines
Quoted 6 lines
Quoted 6 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt new file +1
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid DeviceQuoted 8 lines
Quoted 8 lines
Documentation/acpi/acpi-lid.txt
Quoted 8 lines
Documentation/acpi/acpi-lid.txt new file +8
-0+ +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing a
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid statehas
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lidstate.
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 22
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+However the word of "current" has ambiguity, many AML tablesreturn the lid
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this controlmethod with
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 27
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+cached value, the initial returning value is likely not reliable. Thereare
Quoted 5 lines
Quoted 5 lines
Documentation/acpi/acpi-lid.txt
Quoted 5 lines
Documentation/acpi/acpi-lid.txt new file +5
-0+simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device stateis
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 34
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 35
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows. Sinceit is
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 9 lines
Quoted 9 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 44
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed" notificationto
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod lid
Quoted 17 lines
Quoted 17 lines
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 3 lines
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 4 lines
Quoted 4 lines
how to handle.
Documentation/acpi/acpi-lid.txt line 50
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +During the period the userspace hasn't been switched to use thenew
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 51
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+SW_ACPI_LID event, Linux users can use the following bootparameter to
Quoted 10 lines
Quoted 10 lines
Documentation/acpi/acpi-lid.txt
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 9 lines
Quoted 7 lines
Documentation/acpi/acpi-lid.txt line 62
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have
to
Quoted 1 line
cope with the new
KEY_LID_OPENanyway) would just have to know thatif
Quoted 1 line
switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0) then
it
Quoted 2 lines
can't trust the events and it needs additional heuristics.
I really wished we could leave the kernel alone, but some platform need fixes: we are using an
EV_SW, and on those platform, we only get the close event, which means it gets ignored by the input layer. Those platform have a form factor which makes the situation acceptable (tablet with cover, or very low cost laptops).
However, switching away from a differentEV_SWand tell userspace to ignore it would break systems when you are using a docking station. The acpi would provide fake values for the LID switch and this will screw the session if you are working on a docking station with an external monitor and the LID closed.
Quoted 7 lines
My initial suggestion was:
- detect in the kernel whether the ACPI LID information was judged as reliable
- if reliable keep things as it
- if not reliable add an extraKEY_LID_CLOSEto notify userspace that theSW_LIDis not reliable (it will emulate the LID open events) and that it will only get true close events.
[Lv Zheng]
The problem is:
ACPI subsystem has no idea if it is reliable.
The only possible mean to get this awareness is:
1. Waiting for the user report,
2. Check the AML tables and confirm if this is the problem,
3. Add a quirk in the kernel.
As there could be many such kind of "unreliable tables", the quirk list will be endless. That's the motivation of this discussion:
We need to find a way, end the need of introducing such kind of endless quirk list into the kernel.
The kernel parameter "button.lid_init_state" is a solution to eliminate the need of writing such an endless quirk table. However, it doesn't fix anything.
Some unwanted power consumptions are caused by the userspace behavior.
And it is not fixable in the kernel. That's the motivation for us to discuss here.
Quoted 9 lines
After further thoughts, I think we can simply add the extra key, and have an hwdb entry in logind to enumerate the devices only relying on the close event. If the userspace is not updated, we can tell user to use the button.lid_init_state=open quirk to simulate the behavior logind should expect.
That means only adding one extra key (and its events) in the kernel and let userspace decide what to do.
[Lv Zheng]
Yes. All sounds reasonable.
But TBH, I really do not know how can ACPI subsystem determine the following stuffs automatically:
1. if _LID initial state is reliable or not,
2. will open event be sent by the platform or not.
So my choice will be:
Leaving this to be determined by the users.
We can have a kernel parameter, switching acpi button driver between SW_LID and KEY_LID_CLOSE. IMO, this should be sufficient for the vendors.
Thanks and best regards
-Lv
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
In reply to Dmitry TorokhovHi, Dmitry
Quoted 152 lines
From: Dmitry Torokhov [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 4 lines · Benjamin Tissoires
Hi,
Quoted 1 line · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some
of
Quoted 1 line
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 8 lines
Quoted 8 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Quoted 8 lines
Quoted 8 lines
Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Quoted 4 lines
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0
b/Documentation/acpi/acpi-lid.txtQuoted 6 lines
Quoted 6 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt new file +1
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid DeviceQuoted 8 lines
Documentation/acpi/acpi-lid.txt
Quoted 8 lines
Documentation/acpi/acpi-lid.txt new file +8
-0+ +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing a
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid statehas
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 7 lines
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tablesreturn the lid
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 1 line
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this control methodwith
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device stateis
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 34
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 35
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows. Since itis
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 9 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 44
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed" notificationto
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Documentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod lid
Quoted 17 lines
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 5 lines
how to handle.
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameterto
Quoted 10 lines
Documentation/acpi/acpi-lid.txt
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 5 lines
Documentation/acpi/acpi-lid.txt line 62
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
Quoted 8 lines
Quoted 2 lines
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have to cope with the new
KEY_LID_OPENanyway) would just have to know that if switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0) then it
can't trust the events and it needs additional heuristics.
[Lv Zheng]
I found a problem with the key event approach.
And need your suggestions.
Some AML tables invoke Notify(lid_device, ...) in several different places. It may be invoked from different functions.
Finally, it's not guaranteed that one "lid close" action can only trigger one key close notification. If we use EV_KEY, then there should be many platforms triggering multiple "lid close" events to the user space.
Original switch event based design can automatically eliminate the redundant events.
Does input layer has an event type that can handle such situation? Or shall ACPI button driver handle this?
Thanks,
-Lv
Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Quoted 170 lines · Lv
Hi, Dmitry
Quoted 152 lines
From: Dmitry Torokhov [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 4 lines · Benjamin Tissoires
Hi,
Quoted 1 line · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some
of
Quoted 1 line
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 8 lines
Quoted 8 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Quoted 8 lines
Quoted 8 lines
Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Quoted 4 lines
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0
b/Documentation/acpi/acpi-lid.txtQuoted 6 lines
Quoted 6 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt new file +1
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid DeviceQuoted 8 lines
Documentation/acpi/acpi-lid.txt
Quoted 8 lines
Documentation/acpi/acpi-lid.txt new file +8
-0+ +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing a
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid statehas
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 7 lines
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tablesreturn the lid
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 1 line
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this control methodwith
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device stateis
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 34
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 35
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows. Since itis
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 9 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 44
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed" notificationto
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Documentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod lid
Quoted 17 lines
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 5 lines
how to handle.
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameterto
Quoted 10 lines
Documentation/acpi/acpi-lid.txt
Quoted 10 lines
Documentation/acpi/acpi-lid.txt new file +10
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 5 lines
Documentation/acpi/acpi-lid.txt line 62
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
Quoted 8 lines
Quoted 2 lines
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have to cope with the new
KEY_LID_OPENanyway) would just have to know that if switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0) then it
can't trust the events and it needs additional heuristics.[Lv Zheng]
I found a problem with the key event approach.
And need your suggestions.Some AML tables invoke
Notify(lid_device, ...)in several different places. It may be invoked from different functions.
Quoted 8 lines
Finally, it's not guaranteed that one "lid close" action can only trigger one key close notification. If we use
EV_KEY, then there should be many platforms triggering multiple "lid close" events to the user space.Original switch event based design can automatically eliminate the redundant events.
Does input layer has an event type that can handle such situation? Or shall ACPI button driver handle this?
Keys also have some redundant event elimination, but it's as long as you are holding the key in the press (or released) position. So in your case, that would mean sending input_key(1), wait a little while other notifications are processed, and then sending input_key(0) (assuming each notification comes in with its own thread). Not sure you will gain anything from the new implementation you just sent with the rate-limit.
Cheers,
Benjamin
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Hi, Benjamin
Quoted 206 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 4 lines · Lv
Hi, Dmitry
Quoted 2 lines
From: Dmitry Torokhov [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPIcontrol
Quoted 6 lines
Quoted 6 lines
method lid device restrictions
Quoted 3 lines · Benjamin Tissoires
Hi,
On Thu, Jul 7, 2016 at 9:11 AM, Lv Zheng <[email protected]>
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
There are many AML tables reporting wrong initial lid state, and
some
Quoted 8 lines
Quoted 8 lines
of
Quoted 1 line
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 5 lines
Quoted 5 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event whichis
Quoted 32 lines
Quoted 32 lines
Quoted 11 lines
Quoted 11 lines
reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Quoted 4 lines
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0
b/Documentation/acpi/acpi-lid.txtQuoted 6 lines
Quoted 6 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt new file +1
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid DeviceQuoted 8 lines
Documentation/acpi/acpi-lid.txt
Quoted 8 lines
Documentation/acpi/acpi-lid.txt new file +8
-0+ +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing a
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lidstate
Quoted 7 lines
Quoted 7 lines
has
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lidstate.
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 22
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+However the word of "current" has ambiguity, many AML tablesreturn the lid
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 1 line
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this controlmethod
Quoted 2 lines
Quoted 2 lines
with
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+cached value, the initial returning value is likely not reliable. Thereare
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 28
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+simply so many examples always retuning "closed" as initial lidstate.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid devicestate
Quoted 6 lines
Quoted 6 lines
is
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 34
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows.Since it
Quoted 10 lines
Quoted 10 lines
is
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 2 lines
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 44
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed"notification
Quoted 24 lines
Quoted 24 lines
to
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Documentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod lid
Quoted 17 lines
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 4 lines
how to handle.
Documentation/acpi/acpi-lid.txt line 50
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +During the period the userspace hasn't been switched to use thenew
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 51
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+SW_ACPI_LID event, Linux users can use the following bootparameter
Quoted 4 lines
Quoted 4 lines
to
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linuxkernel
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID controlmethod's
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 2 lines
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have
to
Quoted 1 line
Quoted 1 line
cope with the new
KEY_LID_OPENanyway) would just have to knowthat if
Quoted 1 line
Quoted 1 line
switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0)
then
Quoted 11 lines
Quoted 2 lines
it
can't trust the events and it needs additional heuristics.[Lv Zheng]
I found a problem with the key event approach.
And need your suggestions.Some AML tables invoke
Notify(lid_device, ...)in several different places. It may be invoked from different functions.Finally, it's not guaranteed that one "lid close" action can only trigger
one key close notification.
Quoted 1 line
If we use
EV_KEY, then there should be many platforms triggeringmultiple "lid close" events to the user space.
Quoted 2 lines
Original switch event based design can automatically eliminate the
redundant events.
Quoted 4 lines
Does input layer has an event type that can handle such situation? Or shall ACPI button driver handle this?
Quoted 7 lines
Keys also have some redundant event elimination, but it's as long as you are holding the key in the press (or released) position. So in your case, that would mean sending
input_key(1), wait a little while other notifications are processed, and then sendinginput_key(0)(assuming each notification comes in with its own thread). Not sure you will gain anything from the new implementation you just sent with the rate-limit.
[Lv Zheng]
However this key is virtual.
The multiple notifications are just triggered by the AML code. The Notify(lid_device, xxx) may be invoked in a function.
And this function may be invoked multiple times by other control methods. So I do not know when it is "released".
Using the feature of the keys, it sounds like that I should setup a timer. When the state is changed, I should report input_key(1) and prepare the timer. Then report the input_key(0) when the timer times out.
The side effect is the input_key(0) will be deferred.
I just refreshed the patch as v4 with an ACPI button driver internal workaround. By adding a time_after() check which looks more lightweight than setting up a timer. Could you also help to check if that solution is OK?
Thanks in advance.
Best regards
-Lv
Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Quoted 216 lines · Lv
Hi, Benjamin
Quoted 206 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 4 lines · Lv
Hi, Dmitry
Quoted 2 lines
From: Dmitry Torokhov [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPIcontrol
Quoted 6 lines
Quoted 6 lines
method lid device restrictions
Quoted 3 lines · Benjamin Tissoires
Hi,
On Thu, Jul 7, 2016 at 9:11 AM, Lv Zheng <[email protected]>
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
There are many AML tables reporting wrong initial lid state, and
some
Quoted 8 lines
Quoted 8 lines
of
Quoted 1 line
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 5 lines
Quoted 5 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event whichis
Quoted 32 lines
Quoted 32 lines
Quoted 11 lines
Quoted 11 lines
reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Quoted 4 lines
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0
b/Documentation/acpi/acpi-lid.txtQuoted 6 lines
Quoted 6 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt new file +1
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid DeviceQuoted 8 lines
Documentation/acpi/acpi-lid.txt
Quoted 8 lines
Documentation/acpi/acpi-lid.txt new file +8
-0+ +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing a
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lidstate
Quoted 7 lines
Quoted 7 lines
has
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lidstate.
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 22
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+However the word of "current" has ambiguity, many AML tablesreturn the lid
Quoted 2 lines
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 1 line
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this controlmethod
Quoted 2 lines
Quoted 2 lines
with
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+cached value, the initial returning value is likely not reliable. Thereare
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 28
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+simply so many examples always retuning "closed" as initial lidstate.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid devicestate
Quoted 6 lines
Quoted 6 lines
is
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 34
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows.Since it
Quoted 10 lines
Quoted 10 lines
is
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 2 lines
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Documentation/acpi/acpi-lid.txt line 44
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed"notification
Quoted 24 lines
Quoted 24 lines
to
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Documentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod lid
Quoted 17 lines
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 4 lines
how to handle.
Documentation/acpi/acpi-lid.txt line 50
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +During the period the userspace hasn't been switched to use thenew
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 51
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+SW_ACPI_LID event, Linux users can use the following bootparameter
Quoted 4 lines
Quoted 4 lines
to
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linuxkernel
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID controlmethod's
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 2 lines
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.How about we leave the kernel alone and userspace (which would have
to
Quoted 1 line
Quoted 1 line
cope with the new
KEY_LID_OPENanyway) would just have to knowthat if
Quoted 1 line
Quoted 1 line
switch's parent is PNP0C0D:00 (or phys is PNP0C0D/button/input0)
then
Quoted 11 lines
Quoted 2 lines
it
can't trust the events and it needs additional heuristics.[Lv Zheng]
I found a problem with the key event approach.
And need your suggestions.Some AML tables invoke
Notify(lid_device, ...)in several different places. It may be invoked from different functions.Finally, it's not guaranteed that one "lid close" action can only trigger
one key close notification.
Quoted 1 line
If we use
EV_KEY, then there should be many platforms triggeringmultiple "lid close" events to the user space.
Quoted 2 lines
Original switch event based design can automatically eliminate the
redundant events.
Quoted 4 lines
Does input layer has an event type that can handle such situation? Or shall ACPI button driver handle this?
Quoted 7 lines
Keys also have some redundant event elimination, but it's as long as you are holding the key in the press (or released) position. So in your case, that would mean sending
input_key(1), wait a little while other notifications are processed, and then sendinginput_key(0)(assuming each notification comes in with its own thread). Not sure you will gain anything from the new implementation you just sent with the rate-limit.
Quoted 5 lines
[Lv Zheng]
However this key is virtual.
The multiple notifications are just triggered by the AML code. TheNotify(lid_device, xxx)may be invoked in a function.
And this function may be invoked multiple times by other control methods.
Yes, I understood. In this particular case, when I sad "pressed" or "released" please understand logical 1 or logical 0.
Quoted 1 line
So I do not know when it is "released".
Yep, that's the problem.
Quoted 5 lines
Using the feature of the keys, it sounds like that I should setup a timer. When the state is changed, I should report
input_key(1)and prepare the timer. Then report theinput_key(0)when the timer times out.
The side effect is theinput_key(0)will be deferred.
That's not an issue. If it were an issue, logind can just get triggered when it receives the key(1), and ignore the key(0). But given that the action will likely be power suspend, I don't think even a 500ms delay will be an issue.
Quoted 4 lines
I just refreshed the patch as v4 with an ACPI button driver internal workaround. By adding a
time_after()check which looks more lightweight than setting up a timer. Could you also help to check if that solution is OK?
Yes, that's fine by me. I already sent my rev-by.
Cheers,
Benjamin
Quoted 4 lines
Thanks in advance.
Best regards
-Lv
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
In reply to Benjamin TissoiresHi,
Quoted 77 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsHi,
Quoted 2 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 8 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Quoted 8 lines
Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Documentation/acpi/acpi-
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txtDocumentation/acpi/acpi- renamed from Documentation/acpi/acpi-lid.txt +0
-0lid.txt
Quoted 5 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt line 9
Quoted 9 lines
Documentation/acpi/acpi-lid.txt new file +9
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs usinga
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must beimplemented to
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables returnthe lid
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is itsinitial
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this control methodwith
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state is +changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 14 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 9 lines
Documentation/acpi/acpi-lid.txt new file +9
-0+"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.
[Lv Zheng]
OK.
Quoted 26 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actionsaccording
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisDocumentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI control methodlid
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 4 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspace knows how to handle.
[Lv Zheng]
It should be KEY_LID_CLOSE.
However I checked the KEY code definitions.
It seems their values are highly dependent on the HID specification. I'm not sure which key code value should I use for this. Could you point me out?
Quoted 17 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameterto
Documentation/acpi/acpi-lid.txt line 62
Quoted 11 lines
Documentation/acpi/acpi-lid.txt new file +11
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.
Quoted 5 lines
This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.
[Lv Zheng]
We won't remove SW_LID from the kernel :).
And we haven't removed SW_LID from the acpi button driver.
We'll just stop sending "initial lid state" from acpi button driver, i.e., the behavior carried out by "button.lid_init_state=ignore".
Maybe it is not sufficient, after the userspace has been changed to support the new event, we should stop sending SW_LID from acpi button driver.
Cheers,
-Lv
Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Quoted 1 line · Lv
<snip>
Quoted 10 lines
Quoted 6 lines
This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms,
and not remove it entirely as you will break platforms.[Lv Zheng]
We won't removeSW_LIDfrom the kernel :).
Quoted 7 lines
And we haven't removed
SW_LIDfrom the acpi button driver. We'll just stop sending "initial lid state" from acpi button driver, i.e., the behavior carried out by "button.lid_init_state=ignore".Maybe it is not sufficient, after the userspace has been changed to support the new event, we should stop sending
SW_LIDfrom acpi button driver.
For the affected devices? Sure, but I don't think that's a reasonable thing to do for "all" the devices. We have a majority of laptops where this isn't a problem, and it's not even a problem any more on one of the devices that triggered this discussion (there's a patch for make the LID status match reality for the Surface 3).
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Hi,
Quoted 25 lines
From: Bastien Nocera [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsQuoted 1 line · Lv
<snip>
Quoted 10 lines
Quoted 6 lines
This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms,
and not remove it entirely as you will break platforms.[Lv Zheng]
We won't removeSW_LIDfrom the kernel :).Quoted 7 lines
And we haven't removed
SW_LIDfrom the acpi button driver. We'll just stop sending "initial lid state" from acpi button driver, i.e., the behavior carried out by "button.lid_init_state=ignore".Maybe it is not sufficient, after the userspace has been changed to support the new event, we should stop sending
SW_LIDfrom acpi button driver.
Quoted 5 lines
For the affected devices? Sure, but I don't think that's a reasonable thing to do for "all" the devices. We have a majority of laptops where this isn't a problem, and it's not even a problem any more on one of the devices that triggered this discussion (there's a patch for make the LID status match reality for the Surface 3).
[Lv Zheng]
It looks, even with this fixed, there are tables never generating "lid open" event. Thus the lid notification is definitely not a "switch event".
Thanks and best regards
-Lv
Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
In reply to Zheng, LvQuoted 122 lines · Lv
Hi,
Quoted 77 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsHi,
Quoted 2 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 8 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Quoted 8 lines
Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Documentation/acpi/acpi-
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txtDocumentation/acpi/acpi- renamed from Documentation/acpi/acpi-lid.txt +0
-0lid.txt
Quoted 5 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt line 9
Quoted 9 lines
Documentation/acpi/acpi-lid.txt new file +9
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs usinga
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must beimplemented to
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables returnthe lid
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is itsinitial
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this control methodwith
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state is +changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 14 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 9 lines
Documentation/acpi/acpi-lid.txt new file +9
-0+"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.[Lv Zheng]
OK.Quoted 26 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actionsaccording
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisDocumentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI control methodlid
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 6 lines
Quoted 4 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspace knows how to handle.[Lv Zheng]
It should beKEY_LID_CLOSE.
yep, sorry.
Quoted 2 lines
However I checked the KEY code definitions.
It seems their values are highly dependent on the HID specification.
That was convenient enough when the code was written. Now, we can extend new keycodes as we want, as long as Dmitry agrees :)
Quoted 38 lines
I'm not sure which key code value should I use for this. Could you point me out?
Quoted 17 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameterto
Documentation/acpi/acpi-lid.txt line 62
Quoted 11 lines
Documentation/acpi/acpi-lid.txt new file +11
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.Quoted 5 lines
This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.[Lv Zheng]
We won't removeSW_LIDfrom the kernel :).And we haven't removed
SW_LIDfrom the acpi button driver.
We'll just stop sending "initial lid state" from acpi button driver, i.e., the behavior carried out by "button.lid_init_state=ignore".Maybe it is not sufficient, after the userspace has been changed to support the new event, we should stop sending
SW_LIDfrom acpi button driver.Cheers,
-Lv
Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
[I just realised Ctrl+enter means "send" for gmail, see the end of the answers]
Quoted 130 lines · Benjamin Tissoires
Quoted 122 lines · Lv
Hi,
Quoted 77 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictionsHi,
Quoted 2 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 8 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Quoted 8 lines
Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62+++++++++++++++++++++++++++++++++++++++
Documentation/acpi/acpi-
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txtDocumentation/acpi/acpi- renamed from Documentation/acpi/acpi-lid.txt +0
-0lid.txt
Quoted 5 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt line 9
Quoted 9 lines
Documentation/acpi/acpi-lid.txt new file +9
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs usinga
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must beimplemented to
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables returnthe lid
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is itsinitial
Documentation/acpi/acpi-lid.txt line 26
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this control methodwith
Documentation/acpi/acpi-lid.txt
Quoted 7 lines
Documentation/acpi/acpi-lid.txt new file +7
-0+cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state is +changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 14 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 9 lines
Documentation/acpi/acpi-lid.txt new file +9
-0+"closed" when the lid state is changed to "closed". This is normally used +to trigger some system power saving operations on Windows. Since it is +fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file is only +used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.[Lv Zheng]
OK.Quoted 26 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +New userspace programs should rely on the lid "closed" notification to +trigger some power saving operations and may stop taking actionsaccording
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisDocumentation/acpi/acpi-lid.txt line 47
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI control methodlid
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 6 lines
Quoted 4 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspace knows how to handle.[Lv Zheng]
It should beKEY_LID_CLOSE.
Quoted 12 lines
yep, sorry.
Quoted 2 lines
However I checked the KEY code definitions.
It seems their values are highly dependent on the HID specification.That was convenient enough when the code was written. Now, we can extend new keycodes as we want, as long as Dmitry agrees :)
Quoted 3 lines
I'm not sure which key code value should I use for this. Could you point me out?
I think using 0x277 (or 0x278 given that KEY_DATA is reusing KEY_FASTREVERSE) would be fine.
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
Documentation/acpi/acpi-lid.txt
Quoted 5 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ +During the period the userspace hasn't been switched to use the new +SW_ACPI_LID event, Linux users can use the following boot parameterto
Documentation/acpi/acpi-lid.txt line 62
Quoted 11 lines
Documentation/acpi/acpi-lid.txt new file +11
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linux kernel + reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID control method's + returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LID control + method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.[Lv Zheng]
We won't removeSW_LIDfrom the kernel :).And we haven't removed
SW_LIDfrom the acpi button driver.
We'll just stop sending "initial lid state" from acpi button driver, i.e., the behavior carried out by "button.lid_init_state=ignore".
That won't do for the very same use case than before. It makes sense to boot a laptop while the LID is closed if you have an external monitor plugged in (the docking station allows to have an extra power button accessible when the lid is closed).
Quoted 2 lines
Quoted 2 lines
Maybe it is not sufficient, after the userspace has been changed to support the new event, we should stop sending
SW_LIDfrom acpi button driver.
I'd say do not touch SW_LID at all (and allow users to tweak its behavior for local fixes, which is what you currently have). Just send the extra KEY_LID_CLOSE, no matter what.
And start listing which devices have troubles, and we can add those into a hwdb file shipped with logind. I hope the systemd team will agree with me.
Cheers,
Benjamin
Quoted 3 lines
Quoted 3 lines
Cheers,
-Lv
RE: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions
Hi,
Quoted 166 lines
From: [email protected] [mailto:linux-acpi- [email protected]] On Behalf Of Benjamin Tissoires
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI control method lid device restrictions[I just realised Ctrl+enter means "send" for gmail, see the end of the answers]
Quoted 5 lines · Benjamin Tissoires
Quoted 4 lines · Lv
Hi,
Quoted 2 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPIcontrol
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
method lid device restrictions
Hi,
Quoted 1 line · Lv Zheng
There are many AML tables reporting wrong initial lid state, and
some of
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 1 line
them never reports lid state. As a proxy layer acting between, ACPI
button
Quoted 16 lines
driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.Signed-off-by Lv Zheng <[email protected]>Cc Bastien Nocera: <[email protected]>Cc Benjamin Tissoires <[email protected]>--- Documentation/acpi/acpi-lid.txt | 62Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
+++++++++++++++++++++++++++++++++++++++
Review
Quoted 4 lines
1 file changed, 62 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt(quoted diff) +0
-0b/Documentation/acpi/acpi-
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
lid.txt
Quoted 5 lines
new file mode 100644
index 0000000..7e4f7edDocumentation/acpi/acpi-lid.txt line 9
Quoted 9 lines
Documentation/acpi/acpi-lid.txt new file +9
-0@@ -0,0 +1,62 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMsusing
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
a
Documentation/acpi/acpi-lid.txt line 11
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid statehas
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt line 12
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed. The _LID control method for the lid device must beimplemented to
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of theLinux
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lidstate.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 22
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+However the word of "current" has ambiguity, many AML tablesreturn
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
the lid
Documentation/acpi/acpi-lid.txt line 24
Quoted 2 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the
_LIDQuoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt line 25
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+control method is evaluated during the runtime, the problem is itsinitial
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+returning value. When the AML tables implement this controlmethod
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
with
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+cached value, the initial returning value is likely not reliable. Thereare
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 28
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+simply so many examples always retuning "closed" as initial lidstate.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid devicestate is
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt line 33
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+changed to "opened". But it is ensured that the AML tables alwaysnotify
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+"closed" when the lid state is changed to "closed". This is normallyused
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 35
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to trigger some system power saving operations on Windows.Since it is
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt
Quoted 6 lines
Documentation/acpi/acpi-lid.txt new file +6
-0+fully tested, this notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The userspace programs should stop relying on +/proc/acpi/button/lid/LID0/state to obtain the lid state. This file isonly
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Documentation/acpi/acpi-lid.txt line 42
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+used for the validation purpose.I'd say: this file actually calls the
_LIDmethod described above. And given the previous explanation, it is not reliable enough on some platforms. So it is strongly advised for user-space program to not solely rely on this file to determine the actual lid state.Quoted 6 lines
Quoted 6 lines
[Lv Zheng]
OK.Quoted 3 lines
Documentation/acpi/acpi-lid.txt line 44
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +New userspace programs should rely on the lid "closed"notification to
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Documentation/acpi/acpi-lid.txt line 45
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+trigger some power saving operations and may stop taking actionsaccording
Documentation/acpi/acpi-lid.txt line 46
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+to the lid "opened" notification. A new input switch event -
SW_ACPI_LIDisQuoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+prepared for the new userspace to implement this ACPI controlmethod
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
lid
Documentation/acpi/acpi-lid.txt line 48
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+device specific logics.That's not entirely what we discussed before (to prevent regressions):
- if the device doesn't have reliable LID switch state, then there would be the new input event, and so userspace should only rely on opened notifications.
- if the device has reliable switch information, the new input event should not be exported and userspace knows that the current input switch event is reliable.Also, using a new "switch" event is a terrible idea. Switches have a state (open/close) and you are using this to forward a single open event. So using a switch just allows you to say to userspace you are using the "new" LID meaning, but you'll still have to manually reset the switch and you will have to document how this event is not a switch.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Please use a simple
KEY_LID_OPENevent you will send through [input_key_event(KEY_LID_OPEN, 1),input_sync(),input_key_event(KEY_LID_OPEN, 0),input_sync()], which userspaceknows
Quoted 12 lines
Quoted 3 lines
Quoted 1 line
how to handle.
[Lv Zheng]
It should beKEY_LID_CLOSE.yep, sorry.
Quoted 2 lines
However I checked the KEY code definitions.
It seems their values are highly dependent on the HID specification.That was convenient enough when the code was written. Now, we can extend new keycodes as we want, as long as Dmitry agrees :)
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
I'm not sure which key code value should I use for this. Could you point me out?
I think using
0x277(or0x278given thatKEY_DATAis reusingKEY_FASTREVERSE) would be fine.
[Lv Zheng]
Got it!
Thanks.
Quoted 33 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Documentation/acpi/acpi-lid.txt line 50
Quoted 5 lines
Documentation/acpi/acpi-lid.txt new file +2
-0+ +During the period the userspace hasn't been switched to use thenew
Quoted 1 line
Quoted 1 line
Quoted 1 line
Documentation/acpi/acpi-lid.txt line 51
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+SW_ACPI_LID event, Linux users can use the following bootparameter
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
to
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+handle possible issues: + button.lid_init_state=method: + This is the default behavior of the Linux ACPI lid driver, Linuxkernel
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/acpi/acpi-lid.txt
Quoted 3 lines
Documentation/acpi/acpi-lid.txt new file +3
-0+ reports the initial lid state using the returning value of the _LID + control method. + This can be used to fix some platforms if the _LID controlmethod's
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Documentation/acpi/acpi-lid.txt
Quoted 4 lines
Documentation/acpi/acpi-lid.txt new file +4
-0+ returning value is reliable. + button.lid_init_state=open: + Linux kernel always reports the initial lid state as "opened". + This may fix some platforms if the returning value of the _LIDcontrol
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
Documentation/acpi/acpi-lid.txt line 62
Quoted 1 line
Documentation/acpi/acpi-lid.txt new file +1
-0+ method is not reliable.This worries me as there is no plan after "During the period the userspace hasn't been switched to use the new event".
I really hope you'll keep sending
SW_LIDfor reliable LID platforms, and not remove it entirely as you will break platforms.[Lv Zheng]
We won't removeSW_LIDfrom the kernel :).
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines
And we haven't removed
SW_LIDfrom the acpi button driver. We'll just stop sending "initial lid state" from acpi button driver, i.e.,the behavior carried out by "button.lid_init_state=ignore".
That won't do for the very same use case than before. It makes sense to boot a laptop while the LID is closed if you have an external monitor plugged in (the docking station allows to have an extra power button accessible when the lid is closed).
[Lv Zheng]
Exactly.
The "button.lid_init_state=ignore" is the original behavior of the ACPI button driver.
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Maybe it is not sufficient, after the userspace has been changed to
support the new event, we should stop sending
SW_LIDfrom acpi button driver.
Quoted 6 lines
I'd say do not touch
SW_LIDat all (and allow users to tweak its behavior for local fixes, which is what you currently have). Just send the extraKEY_LID_CLOSE, no matter what.
And start listing which devices have troubles, and we can add those into a hwdb file shipped with logind. I hope the systemd team will agree with me.
[Lv Zheng]
OK.
We'll provide the dmidecode files for those platforms via an off-list way after the agreement is reached. Thanks for the help.
Cheers
-Lv
PATCH v3 1/2 ACPI / button: Add KEY_LID_CLOSE for new usage model
In reply to Lv ZhengThere are many AML tables reporting wrong initial lid state, and some of them never reports lid open state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be an open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds a new input key event so that the new userspace programs can use it to handle this usage model correctly. And in the meanwhile, no old programs will be broken by the userspace changes.
This patch also adds a button.lid_event_type parameter to allow the users to switch between the 2 event types.
--- drivers/acpi/button.c | 109 +++++++++++++++++++++++++------- include/uapi/linux/input-event-codes.h | 6 ++ 2 files changed, 91 insertions(+), 24 deletions(-)
drivers/acpi/button.c +85 -24
@@ -57,6 +57,9 @@#define ACPI_BUTTON_LID_INIT_OPEN 0x01#define ACPI_BUTTON_LID_INIT_METHOD 0x02 +#define ACPI_BUTTON_LID_EVENT_KEY 0x00 +#define ACPI_BUTTON_LID_EVENT_SWITCH 0x01 +#define _COMPONENT ACPI_BUTTON_COMPONENTACPI_MODULE_NAME ("button" );
@@ -110,6 +113,7 @@ struct acpi_button {static BLOCKING_NOTIFIER_HEAD (acpi_lid_notifier);static struct acpi_device *lid_device;static u8 lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; +static u8 lid_event_type =ACPI_BUTTON_LID_EVENT_SWITCH ;/* -------------------------------------------------------------------------- FS Interface (/proc)
@@ -136,8 +140,17 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)int ret;/* input layer checks if event is redundant */ -input_report_switch (button->input,SW_LID , !state); -input_sync (button->input); +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_SWITCH ) { +input_report_switch (button->input,SW_LID , !state); +input_sync (button->input); + } +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_KEY && + !state) { +input_report_key (button->input,KEY_LID_CLOSE ,1 ); +input_sync (button->input); +input_report_key (button->input,KEY_LID_CLOSE ,0 ); +input_sync (button->input); + }if (state)pm_wakeup_event (&device->dev,0 );
@@ -292,6 +305,9 @@ static int acpi_lid_update_state(struct acpi_device *device)static void acpi_lid_initialize_state (struct acpi_device *device) { +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_KEY ) +return ; +switch (lid_init_state) {case ACPI_BUTTON_LID_INIT_OPEN : (void )acpi_lid_notify_state (device,1 );
@@ -436,6 +452,7 @@ static int acpi_button_add(struct acpi_device *device)case ACPI_BUTTON_TYPE_LID :input_set_capability (input,EV_SW ,SW_LID ); +input_set_capability (input,EV_KEY ,KEY_LID_CLOSE );break ; }
@@ -475,35 +492,49 @@ static int acpi_button_remove(struct acpi_device *device)static int param_set_lid_init_state (const char *val,struct kernel_param *kp) { -int result =0 ; - -if (!strncmp (val,"open" ,sizeof ("open" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_OPEN ; -pr_info ("Notify initial lid state as open\n" ); - }else if (!strncmp (val,"method" ,sizeof ("method" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; -pr_info ("Notify initial lid state with _LID return value\n" ); - }else if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; -pr_info ("Do not notify initial lid state\n" ); - }else - result = -EINVAL ; +int result = -EINVAL ; + +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY : +if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; +pr_info ("Do not notify initial lid state\n" ); + } +break ; +case ACPI_BUTTON_LID_EVENT_SWITCH : +if (!strncmp (val,"open" ,sizeof ("open" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_OPEN ; +pr_info ("Notify initial lid state as open\n" ); + }else if (!strncmp (val,"method" ,sizeof ("method" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; +pr_info ("Notify initial lid state" +" with _LID return value\n" ); + }else if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; +pr_info ("Do not notify initial lid state\n" ); + } +break ; + }return result; }static int param_get_lid_init_state (char *buffer,struct kernel_param *kp) { -switch (lid_init_state) { -case ACPI_BUTTON_LID_INIT_OPEN : -return sprintf (buffer,"open" ); -case ACPI_BUTTON_LID_INIT_METHOD : -return sprintf (buffer,"method" ); -case ACPI_BUTTON_LID_INIT_IGNORE : +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY :return sprintf (buffer,"ignore" ); -default : -return sprintf (buffer,"invalid" ); +case ACPI_BUTTON_LID_EVENT_SWITCH : +switch (lid_init_state) { +case ACPI_BUTTON_LID_INIT_OPEN : +return sprintf (buffer,"open" ); +case ACPI_BUTTON_LID_INIT_METHOD : +return sprintf (buffer,"method" ); +case ACPI_BUTTON_LID_INIT_IGNORE : +return sprintf (buffer,"ignore" ); + } +break ; } -return 0 ; +return sprintf (buffer,"invalid" ); } module_param_call(lid_init_state,
@@ -511,4 +542,34 @@ module_param_call(lid_init_state, NULL, 0644);MODULE_PARM_DESC (lid_init_state,"Behavior for reporting LID initial state" ); +static int param_set_lid_event_type (const char *val,struct kernel_param *kp) +{ +int result = -EINVAL ; + +if (!strncmp (val,"key" ,sizeof ("key" ) -1 )) { + lid_event_type =ACPI_BUTTON_LID_EVENT_KEY ; +pr_info ("Notify lid state using key event\n" ); + }else if (!strncmp (val,"switch" ,sizeof ("switch" ) -1 )) { + lid_event_type =ACPI_BUTTON_LID_EVENT_SWITCH ; +pr_info ("Notify lid state using switch event\n" ); + } +return result; +} + +static int param_get_lid_event_type (char *buffer,struct kernel_param *kp) +{ +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY : +return sprintf (buffer,"key" ); +case ACPI_BUTTON_LID_EVENT_SWITCH : +return sprintf (buffer,"switch" ); + } +return sprintf (buffer,"invalid" ); +} + +module_param_call (lid_event_type, + param_set_lid_event_type, param_get_lid_event_type, +NULL ,0644 ); +MODULE_PARM_DESC (lid_event_type,"Event type for reporting LID state" ); +module_acpi_driver (acpi_button_driver);
include/uapi/linux/input-event-codes.h +6 -0
@@ -641,6 +641,12 @@* e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.) */ #define KEY_DATA 0x275 +/* +* Special event sent by the lid drivers. The drivers may not be able to +* issue "open" event, in which case, they send KEY_LID_CLOSE instead of +* SW_LID. +*/ +#define KEY_LID_CLOSE 0x278#define BTN_TRIGGER_HAPPY 0x2c0#define BTN_TRIGGER_HAPPY1 0x2c0
Signature · 1 line
1.7.10
Re: [PATCH v3 1/2] ACPI / button: Add KEY_LID_CLOSE for new usage model
Hi,
Quoted 5 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid open state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
Quoted 8 lines
2. There may not be an open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds a new input key event so that the new userspace programs can use it to handle this usage model correctly. And in the meanwhile, no old programs will be broken by the userspace changes.
This patch also adds a button.lid_event_type parameter to allow the users to switch between the 2 event types.
I think we are getting closer to what would be acceptable for user-space, but I still have a few comments:
Quoted 64 lines
Signed-off-by Lv Zheng <[email protected]>Cc Dmitry Torokhov <[email protected]>Cc Benjamin Tissoires <[email protected]>Cc Bastien Nocera: <[email protected]>--- drivers/acpi/button.c | 109 +++++++++++++++++++++++++------- include/uapi/linux/input-event-codes.h | 6 ++ 2 files changed, 91 insertions(+), 24 deletions(-)drivers/acpi/button.c +18
-2@@ -57,6 +57,9 @@#define ACPI_BUTTON_LID_INIT_OPEN 0x01#define ACPI_BUTTON_LID_INIT_METHOD 0x02 +#define ACPI_BUTTON_LID_EVENT_KEY 0x00 +#define ACPI_BUTTON_LID_EVENT_SWITCH 0x01 +#define _COMPONENT ACPI_BUTTON_COMPONENTACPI_MODULE_NAME ("button" );@@ -110,6 +113,7 @@ struct acpi_button {static BLOCKING_NOTIFIER_HEAD (acpi_lid_notifier);static struct acpi_device *lid_device;static u8 lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; +static u8 lid_event_type =ACPI_BUTTON_LID_EVENT_SWITCH ;/* -------------------------------------------------------------------------- FS Interface (/proc) @@ -136,8 +140,17 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)int ret;/* input layer checks if event is redundant */ -input_report_switch (button->input,SW_LID , !state); -input_sync (button->input); +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_SWITCH ) { +input_report_switch (button->input,SW_LID , !state); +input_sync (button->input); + } +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_KEY && + !state) { +input_report_key (button->input,KEY_LID_CLOSE ,1 ); +input_sync (button->input); +input_report_key (button->input,KEY_LID_CLOSE ,0 ); +input_sync (button->input); + }if (state)pm_wakeup_event (&device->dev,0 );@@ -292,6 +305,9 @@ static int acpi_lid_update_state(struct acpi_device *device)static void acpi_lid_initialize_state (struct acpi_device *device) { +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_KEY ) +return ; +switch (lid_init_state) {case ACPI_BUTTON_LID_INIT_OPEN : (void )acpi_lid_notify_state (device,1 );
drivers/acpi/button.c static int acpi_button_add(struct acpi_device *device) · line 455
Quoted 4 lines
drivers/acpi/button.c +1
-0@@ -436,6 +452,7 @@ static int acpi_button_add(struct acpi_device *device)case ACPI_BUTTON_TYPE_LID :input_set_capability (input,EV_SW ,SW_LID ); +input_set_capability (input,EV_KEY ,KEY_LID_CLOSE );
Here you are basically exporting the 2 input events but only update one of the 2. It will confuse userspace and it is generally better not to export unused input codes.
However, as I'll say below, I think we should keep the code that way here.
Quoted 98 lines
drivers/acpi/button.c +55
-22break ; }@@ -475,35 +492,49 @@ static int acpi_button_remove(struct acpi_device *device)static int param_set_lid_init_state (const char *val,struct kernel_param *kp) { -int result =0 ; - -if (!strncmp (val,"open" ,sizeof ("open" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_OPEN ; -pr_info ("Notify initial lid state as open\n" ); - }else if (!strncmp (val,"method" ,sizeof ("method" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; -pr_info ("Notify initial lid state with _LID return value\n" ); - }else if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; -pr_info ("Do not notify initial lid state\n" ); - }else - result = -EINVAL ; +int result = -EINVAL ; + +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY : +if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; +pr_info ("Do not notify initial lid state\n" ); + } +break ; +case ACPI_BUTTON_LID_EVENT_SWITCH : +if (!strncmp (val,"open" ,sizeof ("open" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_OPEN ; +pr_info ("Notify initial lid state as open\n" ); + }else if (!strncmp (val,"method" ,sizeof ("method" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; +pr_info ("Notify initial lid state" +" with _LID return value\n" ); + }else if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; +pr_info ("Do not notify initial lid state\n" ); + } +break ; + }return result; }static int param_get_lid_init_state (char *buffer,struct kernel_param *kp) { -switch (lid_init_state) { -case ACPI_BUTTON_LID_INIT_OPEN : -return sprintf (buffer,"open" ); -case ACPI_BUTTON_LID_INIT_METHOD : -return sprintf (buffer,"method" ); -case ACPI_BUTTON_LID_INIT_IGNORE : +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY :return sprintf (buffer,"ignore" ); -default : -return sprintf (buffer,"invalid" ); +case ACPI_BUTTON_LID_EVENT_SWITCH : +switch (lid_init_state) { +case ACPI_BUTTON_LID_INIT_OPEN : +return sprintf (buffer,"open" ); +case ACPI_BUTTON_LID_INIT_METHOD : +return sprintf (buffer,"method" ); +case ACPI_BUTTON_LID_INIT_IGNORE : +return sprintf (buffer,"ignore" ); + } +break ; } -return 0 ; +return sprintf (buffer,"invalid" ); } module_param_call(lid_init_state,@@ -511,4 +542,34 @@ module_param_call(lid_init_state, NULL, 0644);MODULE_PARM_DESC (lid_init_state,"Behavior for reporting LID initial state" ); +static int param_set_lid_event_type (const char *val,struct kernel_param *kp) +{ +int result = -EINVAL ; + +if (!strncmp (val,"key" ,sizeof ("key" ) -1 )) { + lid_event_type =ACPI_BUTTON_LID_EVENT_KEY ; +pr_info ("Notify lid state using key event\n" ); + }else if (!strncmp (val,"switch" ,sizeof ("switch" ) -1 )) { + lid_event_type =ACPI_BUTTON_LID_EVENT_SWITCH ; +pr_info ("Notify lid state using switch event\n" ); + } +return result; +} + +static int param_get_lid_event_type (char *buffer,struct kernel_param *kp) +{ +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY : +return sprintf (buffer,"key" );
drivers/acpi/button.c module_param_call(lid_init_state, · line 573
Quoted 10 lines
drivers/acpi/button.c +10
-0+case ACPI_BUTTON_LID_EVENT_SWITCH : +return sprintf (buffer,"switch" ); + } +return sprintf (buffer,"invalid" ); +} + +module_param_call (lid_event_type, + param_set_lid_event_type, param_get_lid_event_type, +NULL ,0644 ); +MODULE_PARM_DESC (lid_event_type,"Event type for reporting LID state" );
I don't think this is a good solution to have a kernel parameter. I thought the final decision were to have userspace decide which event was valid, and so we just need to export and emit both of events.
_If_ you export a kernel parameter, it makes sense to have a dmi blacklist to have a good default experience, which is what you wanted to avoid.
So if you just export and use both events at the same time, you will have:
- correct ACPI machines will just have an extra KEY_LID_CLOSE event emitted, which will not harm logind
- wrong ACPI machines will not have their SW_LID input event updated because it will be kept closed. But given that logind will ignore it, there is no harm either
As Dmitry said, we could also have a KEY_LID_OPEN emitted for symmetrical purposes, but I am not entirely sure if this will confuse userspace or not. On the other hand, there are few users of these states, and we can teach them how to properly use them.
So in the end, I think you should just get rid of the kernel parameter, export SW_LID, KEY_LID_CLOSE, KEY_LID_OPEN in the event node, and only add the KEY_LID_CLOSE|OPEN events on an actual acpi notification.
Then a small hwdb entry set will teach logind/powerd if they need to ignore the SW_LID event or not.
Cheers,
Benjamin
Quoted 22 lines
drivers/acpi/button.c +1
-0+module_acpi_driver (acpi_button_driver);include/uapi/linux/input-event-codes.h +6
-1@@ -641,6 +641,12 @@* e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.) */ #define KEY_DATA 0x275 +/* +* Special event sent by the lid drivers. The drivers may not be able to +* issue "open" event, in which case, they send KEY_LID_CLOSE instead of +* SW_LID. +*/ +#define KEY_LID_CLOSE 0x278#define BTN_TRIGGER_HAPPY 0x2c0#define BTN_TRIGGER_HAPPY1 0x2c0 --1.7.10
Re: [PATCH v3 1/2] ACPI / button: Add KEY_LID_CLOSE for new usage model
Quoted 1 line · Benjamin Tissoires
<snip>
Quoted 20 lines
I don't think this is a good solution to have a kernel parameter. I thought the final decision were to have userspace decide which event was valid, and so we just need to export and emit both of events.
_If_ you export a kernel parameter, it makes sense to have a dmi blacklist to have a good default experience, which is what you wanted to avoid.
So if you just export and use both events at the same time, you will have:
- correct ACPI machines will just have an extraKEY_LID_CLOSEevent emitted, which will not harm logind
- wrong ACPI machines will not have theirSW_LIDinput event updated because it will be kept closed. But given that logind will ignore it, there is no harm eitherAs Dmitry said, we could also have a
KEY_LID_OPENemitted for symmetrical purposes, but I am not entirely sure if this will confuse userspace or not. On the other hand, there are few users of these states, and we can teach them how to properly use them.
Quoted 7 lines
So in the end, I think you should just get rid of the kernel parameter, export
SW_LID,KEY_LID_CLOSE,KEY_LID_OPENin the event node, and only add the KEY_LID_CLOSE|OPEN events on an actual acpi notification.Then a small hwdb entry set will teach logind/powerd if they need to ignore the
SW_LIDevent or not.
So user-space would have its own blacklist (likely in udev through an hwdb), instead of having it in the kernel? That seems like a fine idea to me, and one of the first consumers, logind, would have all the necessary data straight away.
RE: [PATCH v3 1/2] ACPI / button: Add KEY_LID_CLOSE for new usage model
In reply to Benjamin TissoiresHi, Benjamin
Quoted 17 lines
From: Benjamin Tissoires [mailto:[email protected]]
Subject: Re: [PATCH v3 1/2] ACPI / button: AddKEY_LID_CLOSEfor new usage modelHi,
Quoted 9 lines · Lv Zheng
There are many AML tables reporting wrong initial lid state, and some of them never reports lid open state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be an open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds a new input key event so that the new userspaceprograms
Quoted 8 lines
Quoted 3 lines
can use it to handle this usage model correctly. And in the meanwhile, no old programs will be broken by the userspace changes.
This patch also adds a button.lid_event_type parameter to allow theusers
Quoted 1 line
to switch between the 2 event types.
I think we are getting closer to what would be acceptable for user-space, but I still have a few comments:
[Lv Zheng]
OK.
Quoted 70 lines
Quoted 9 lines
Signed-off-by Lv Zheng <[email protected]>Cc Dmitry Torokhov <[email protected]>Cc Benjamin Tissoires <[email protected]>Cc Bastien Nocera: <[email protected]>--- drivers/acpi/button.c | 109 +++++++++++++++++++++++++-------
drivers/acpi/button.c static int acpi_lid_notify_state(struct
Quoted 26 lines
include/uapi/linux/input-event-codes.h | 6 ++ 2 files changed, 91 insertions(+), 24 deletions(-)drivers/acpi/button.c +4
-0@@ -57,6 +57,9 @@#define ACPI_BUTTON_LID_INIT_OPEN 0x01#define ACPI_BUTTON_LID_INIT_METHOD 0x02 +#define ACPI_BUTTON_LID_EVENT_KEY 0x00 +#define ACPI_BUTTON_LID_EVENT_SWITCH 0x01 +#define _COMPONENT ACPI_BUTTON_COMPONENTACPI_MODULE_NAME ("button" );@@ -110,6 +113,7 @@ struct acpi_button {static BLOCKING_NOTIFIER_HEAD (acpi_lid_notifier);static struct acpi_device *lid_device;static u8 lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; +static u8 lid_event_type =ACPI_BUTTON_LID_EVENT_SWITCH ;/* -------------------------------------------------------------------------- FS Interface (/proc) @@ -136,8 +140,17 @@ static int acpi_lid_notify_state(struct
acpi_device*device, int state)drivers/acpi/button.c static int acpi_lid_update_state(struct
Quoted 20 lines
drivers/acpi/button.c +11
-2int ret;/* input layer checks if event is redundant */ -input_report_switch (button->input,SW_LID , !state); -input_sync (button->input); +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_SWITCH ) { +input_report_switch (button->input,SW_LID , !state); +input_sync (button->input); + } +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_KEY && + !state) { +input_report_key (button->input,KEY_LID_CLOSE ,1 ); +input_sync (button->input); +input_report_key (button->input,KEY_LID_CLOSE ,0 ); +input_sync (button->input); + }if (state)pm_wakeup_event (&device->dev,0 );@@ -292,6 +305,9 @@ static int acpi_lid_update_state(struct
acpi_device*device)drivers/acpi/button.c static int acpi_button_add(struct acpi_device
Quoted 10 lines
drivers/acpi/button.c +3
-0static void acpi_lid_initialize_state (struct acpi_device *device) { +if (lid_event_type ==ACPI_BUTTON_LID_EVENT_KEY ) +return ; +switch (lid_init_state) {case ACPI_BUTTON_LID_INIT_OPEN : (void )acpi_lid_notify_state (device,1 );@@ -436,6 +452,7 @@ static int acpi_button_add(struct acpi_device*device)
Quoted 1 line
Quoted 7 lines
Quoted 3 lines
case ACPI_BUTTON_TYPE_LID: input_set_capability(input, EV_SW, SW_LID);+
input_set_capability(input, EV_KEY, KEY_LID_CLOSE);Here you are basically exporting the 2 input events but only update one of the 2. It will confuse userspace and it is generally better not to export unused input codes.
[Lv Zheng]
I just do not know if it is proper to clear the capability during runtime via module parameters.
Quoted 2 lines
However, as I'll say below, I think we should keep the code that way here.
[Lv Zheng]
Great. So we needn't think about input_clear_capability().
Quoted 118 lines
drivers/acpi/button.c static int acpi_button_remove(struct
Quoted 6 lines
break; }drivers/acpi/button.c +0
-0@@ -475,35 +492,49 @@ static int acpi_button_remove(struct
acpi_device*device)drivers/acpi/button.c static int acpi_button_remove(struct · line 492
Quoted 2 lines
drivers/acpi/button.c +0
-0static int param_set_lid_init_state(const char *val, struct kernel_param*kp)
Quoted 31 lines
drivers/acpi/button.c +17
-13{ -int result =0 ; - -if (!strncmp (val,"open" ,sizeof ("open" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_OPEN ; -pr_info ("Notify initial lid state as open\n" ); - }else if (!strncmp (val,"method" ,sizeof ("method" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; -pr_info ("Notify initial lid state with _LID return value\n" ); - }else if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { - lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; -pr_info ("Do not notify initial lid state\n" ); - }else - result = -EINVAL ; +int result = -EINVAL ; + +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY : +if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; +pr_info ("Do not notify initial lid state\n" ); + } +break ; +case ACPI_BUTTON_LID_EVENT_SWITCH : +if (!strncmp (val,"open" ,sizeof ("open" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_OPEN ; +pr_info ("Notify initial lid state as open\n" ); + }else if (!strncmp (val,"method" ,sizeof ("method" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_METHOD ; +pr_info ("Notify initial lid state" +" with _LID return value\n" );drivers/acpi/button.c static int acpi_button_remove(struct acpi_device *device)
Quoted 10 lines
drivers/acpi/button.c +6
-0+ }else if (!strncmp (val,"ignore" ,sizeof ("ignore" ) -1 )) { + lid_init_state =ACPI_BUTTON_LID_INIT_IGNORE ; +pr_info ("Do not notify initial lid state\n" ); + } +break ; + }return result; } static int param_get_lid_init_state(char *buffer, struct kernel_param*kp)
Quoted 29 lines
drivers/acpi/button.c +13
-9{ -switch (lid_init_state) { -case ACPI_BUTTON_LID_INIT_OPEN : -return sprintf (buffer,"open" ); -case ACPI_BUTTON_LID_INIT_METHOD : -return sprintf (buffer,"method" ); -case ACPI_BUTTON_LID_INIT_IGNORE : +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY :return sprintf (buffer,"ignore" ); -default : -return sprintf (buffer,"invalid" ); +case ACPI_BUTTON_LID_EVENT_SWITCH : +switch (lid_init_state) { +case ACPI_BUTTON_LID_INIT_OPEN : +return sprintf (buffer,"open" ); +case ACPI_BUTTON_LID_INIT_METHOD : +return sprintf (buffer,"method" ); +case ACPI_BUTTON_LID_INIT_IGNORE : +return sprintf (buffer,"ignore" ); + } +break ; } -return 0 ; +return sprintf (buffer,"invalid" ); } module_param_call(lid_init_state,drivers/acpi/button.c module_param_call(lid_init_state, · line 543
Quoted 2 lines
drivers/acpi/button.c +0
-0@@ -511,4 +542,34 @@ module_param_call(lid_init_state, NULL, 0644); MODULE_PARM_DESC(lid_init_state, "Behavior for reporting LID initialstate");
drivers/acpi/button.c module_param_call(lid_init_state, · line 544
Quoted 2 lines
drivers/acpi/button.c +1
-0+static int param_set_lid_event_type(const char *val, struct
kernel_param*kp)Quoted 4 lines
drivers/acpi/button.c +4
-0+{ +int result = -EINVAL ; + +if (!strncmp (val,"key" ,sizeof ("key" ) -1 )) {drivers/acpi/button.c module_param_call(lid_init_state,
Quoted 10 lines
drivers/acpi/button.c +10
-0+ lid_event_type =ACPI_BUTTON_LID_EVENT_KEY ; +pr_info ("Notify lid state using key event\n" ); + }else if (!strncmp (val,"switch" ,sizeof ("switch" ) -1 )) { + lid_event_type =ACPI_BUTTON_LID_EVENT_SWITCH ; +pr_info ("Notify lid state using switch event\n" ); + } +return result; +} + +static int param_get_lid_event_type(char *buffer, struct kernel_param*kp)
Quoted 4 lines
drivers/acpi/button.c +4
-0+{ +switch (lid_event_type) { +case ACPI_BUTTON_LID_EVENT_KEY : +return sprintf (buffer,"key" );drivers/acpi/button.c module_param_call(lid_init_state,
Quoted 10 lines
drivers/acpi/button.c +10
-0+case ACPI_BUTTON_LID_EVENT_SWITCH : +return sprintf (buffer,"switch" ); + } +return sprintf (buffer,"invalid" ); +} + +module_param_call (lid_event_type, + param_set_lid_event_type, param_get_lid_event_type, +NULL ,0644 ); +MODULE_PARM_DESC(lid_event_type, "Event type for reporting LIDstate");
Quoted 3 lines
I don't think this is a good solution to have a kernel parameter. I thought the final decision were to have userspace decide which event was valid, and so we just need to export and emit both of events.
[Lv Zheng]
OK.
Quoted 4 lines
_If_ you export a kernel parameter, it makes sense to have a dmi blacklist to have a good default experience, which is what you wanted to avoid.
[Lv Zheng]
I was thinking that the distribution vendors can configure a default boot parameters for a specific hardware.
Quoted 6 lines
So if you just export and use both events at the same time, you will have:
- correct ACPI machines will just have an extraKEY_LID_CLOSEevent emitted, which will not harm logind
- wrong ACPI machines will not have theirSW_LIDinput event updated because it will be kept closed. But given that logind will ignore it, there is no harm either
[Lv Zheng]
I see.
Quoted 10 lines
As Dmitry said, we could also have a
KEY_LID_OPENemitted for symmetrical purposes, but I am not entirely sure if this will confuse userspace or not. On the other hand, there are few users of these states, and we can teach them how to properly use them.So in the end, I think you should just get rid of the kernel parameter, export
SW_LID,KEY_LID_CLOSE,KEY_LID_OPENin the event node, and only add the KEY_LID_CLOSE|OPEN events on an actual acpi notification.
[Lv Zheng]
I'm sorry I didn't notice the KEY_LID_OPEN comment. Thanks for the reminder.
Quoted 3 lines
Then a small hwdb entry set will teach logind/powerd if they need to ignore the
SW_LIDevent or not.
[Lv Zheng]
OK, I'll address what you've suggested in this email.
Thanks and best regards
-Lv
Quoted 28 lines
Cheers,
BenjaminReview
Quoted 3 lines
drivers/acpi/button.c +1
-0+module_acpi_driver (acpi_button_driver);(quoted diff) +0
-0
b/include/uapi/linux/input-event-codes.hinclude/uapi/linux/input-event-codes.h line 646
Quoted 10 lines
index 737fa32..df7c0c0 100644
include/uapi/linux/input-event-codes.h +3
-0@@ -641,6 +641,12 @@* e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.) */ #define KEY_DATA 0x275 +/* +* Special event sent by the lid drivers. The drivers may not be able to +* issue "open" event, in which case, they send KEY_LID_CLOSE instead of
Quoted 9 lines
include/uapi/linux/input-event-codes.h +3
-1+* SW_LID. +*/ +#define KEY_LID_CLOSE 0x278#define BTN_TRIGGER_HAPPY 0x2c0#define BTN_TRIGGER_HAPPY1 0x2c0 --1.7.10
PATCH v3 2/2 ACPI / button: Add document for ACPI control method lid device restrictions
In reply to Lv ZhengThere are many AML tables reporting wrong initial lid state, and some of them never reports lid open state. As a proxy layer acting between, ACPI button driver is not able to handle all such cases, but need to re-define the usage model of the ACPI lid. That is:
1. It's initial state is not reliable;
2. There may not be an open event;
3. Userspace should only take action against the close event which is reliable, always sent after a real lid close.
This patch adds documentation of the usage model.
--- Documentation/acpi/acpi-lid.txt | 89 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 Documentation/acpi/acpi-lid.txt
Documentation/acpi/acpi-lid.txt new file +89 -0
@@ -0,0 +1,89 @@ +Usage Model of the ACPI Control Method Lid Device + +Copyright (C) 2016, Intel Corporation +Author: Lv Zheng <[email protected]> + + +Abstract: + +Platforms containing lids convey lid state (open/close) to OSPMs using a +control method lid device. To implement this, the AML tables issue +Notify(lid_device, 0x80) to notify the OSPMs whenever the lid state has +changed. The _LID control method for the lid device must be implemented to +report the "current" state of the lid as either "opened" or "closed". + +This document describes the restrictions and the expections of the Linux +ACPI lid device driver. + + +1. Restrictions of the returning value of the _LID control method + +The _LID control method is described to return the "current" lid state. +However the word of "current" has ambiguity, many AML tables return the lid +state upon the last lid notification instead of returning the lid state +upon the last _LID evaluation. There won't be difference when the _LID +control method is evaluated during the runtime, the problem is its initial +returning value. When the AML tables implement this control method with +cached value, the initial returning value is likely not reliable. There are +simply so many examples always retuning "closed" as initial lid state. + +2. Restrictions of the lid state change notifications + +There are many AML tables never notifying when the lid device state is +changed to "opened". Thus the "opened" notification is not guaranteed. + +But it is guaranteed that the AML tables always notify "closed" when the +lid state is changed to "closed". The "closed" notification is normally +used to trigger some system power saving operations on Windows. Since it is +fully tested, the "closed" notification is reliable for all AML tables. + +3. Expections for the userspace users of the ACPI lid device driver + +The ACPI button driver exports the lid state to the userspace via the +following file: + /proc/acpi/button/lid/LID0/state +This file actually calls the _LID control method described above. And given +the previous explanation, it is not reliable enough on some platforms. So +it is advised for the userspace program to not to solely rely on this file +to determine the actual lid state. + +Linux users can switch the ACPI button driver behavior via the following +kernel parameters: +A. button.lid_event_type=switch: + When the lid state/event is reliable, the users can specify this option + (or nothing as this is the default option) to load the ACPI button + driver. The ACPI button driver will send the traditional "SW_LID" event + to the userspace. +B. button.lid_event_type=key: + When the lid state/event is not reliable, the users can specify this + option to load the ACPI button driver. The ACPI button driver will send + the "KEY_LID_CLOSE" event instead of the "SW_LID" to the userspace. + +If the userspace hasn't been prepared to handle the new KEY_LID_CLOSE +event, Linux users can use the following kernel parameters to handle the +possible issues triggered because of the unreliable lid state/event: +C. button.lid_init_state=method: + This option is only effective when the event type is "switch". When this + option is specified, the ACPI button driver reports the initial lid + state using the returning value of the _LID control method. + This option can be used to fix some platforms where the _LID control + method's returning value is reliable but the initial lid state + notification is missing. +D. button.lid_init_state=open: + This option is only effective when the event type is "switch". When this + option is specified, the ACPI button driver always reports the initial + lid state as "opened". + This may fix some platforms where the returning value of the _LID + control method is not reliable and the initial lid state notification is + missing. + +If the userspace has been prepared to handle the new KEY_LID_CLOSE event, +Linux users should always use the following kernel parameter: +E. button.lid_init_state=ignore: + This option allows the users to switch the "event type" between the + "switch" and the "key". When this option is specified, the ACPI button + driver never reports the initial lid state. However, the platform may + automatically report a correct initial lid state and there is no "open" + event missing. When this is the case (everything is correctly + implemented by the platform firmware), the "event type" should be + "switch", otherwise, the "event type" should be "key".
Signature · 1 line
1.7.10