Hi, > From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi- > owner@xxxxxxxxxxxxxxx] 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] > > On Mon, Jul 11, 2016 at 1:42 PM, Benjamin Tissoires > <benjamin.tissoires@xxxxxxxxx> wrote: > > On Mon, Jul 11, 2016 at 5:20 AM, Zheng, Lv <lv.zheng@xxxxxxxxx> wrote: > >> Hi, > >> > >>> From: Benjamin Tissoires [mailto:benjamin.tissoires@xxxxxxxxx] > >>> Subject: Re: [PATCH v2 4/4] ACPI / button: Add document for ACPI > control > >>> method lid device restrictions > >>> > >>> Hi, > >>> > >>> On Thu, Jul 7, 2016 at 9:11 AM, Lv Zheng <lv.zheng@xxxxxxxxx> wrote: > >>> > 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. > >>> > > >>> > Link: https://lkml.org/2016/3/7/460 > >>> > Link: https://github.com/systemd/systemd/issues/2087 > >>> > Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx> > >>> > Cc: Bastien Nocera: <hadess@xxxxxxxxxx> > >>> > Cc: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx> > >>> > Cc: linux-input@xxxxxxxxxxxxxxx > >>> > --- > >>> > Documentation/acpi/acpi-lid.txt | 62 > >>> +++++++++++++++++++++++++++++++++++++++ > >>> > 1 file changed, 62 insertions(+) > >>> > create mode 100644 Documentation/acpi/acpi-lid.txt > >>> > > >>> > diff --git a/Documentation/acpi/acpi-lid.txt > b/Documentation/acpi/acpi- > >>> lid.txt > >>> > new file mode 100644 > >>> > index 0000000..7e4f7ed > >>> > --- /dev/null > >>> > +++ b/Documentation/acpi/acpi-lid.txt > >>> > @@ -0,0 +1,62 @@ > >>> > +Usage Model of the ACPI Control Method Lid Device > >>> > + > >>> > +Copyright (C) 2016, Intel Corporation > >>> > +Author: Lv Zheng <lv.zheng@xxxxxxxxx> > >>> > + > >>> > + > >>> > +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. > >>> > >>> 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. > >> [Lv Zheng] > >> OK. > >> > >>> > >>> > + > >>> > +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. > >> [Lv Zheng] > >> It should be KEY_LID_CLOSE. > > > > yep, sorry. > > > >> 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 :) > > > >> 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. [Lv Zheng] Got it! Thanks. > > > > >>> > >>> > + > >>> > +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. > >>> > >>> 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. > >> > >> [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". > > 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. > > >> > >> 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. > > 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. [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 ��.n��������+%������w��{.n�����{��)��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
