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. 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? > > > + > > +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". 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 ��.n��������+%������w��{.n�����{��)��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
