DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CD24B7E9D9
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Lv Zheng <lv.zheng@intel.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        "Rafael J . Wysocki" <rjw@rjwysocki.net>,
        Len Brown <len.brown@intel.com>, Lv Zheng <zetalog@gmail.com>,
        Peter Hutterer <peter.hutterer@who-t.net>
Cc: linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
        systemd-devel@lists.freedesktop.org, linux-input@vger.kernel.org,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>
Subject: [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI
Date: Thu,  1 Jun 2017 20:46:28 +0200
Message-Id: <20170601184632.2980-1-benjamin.tissoires@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4037
Lines: 88

Hi,

Sending this as a WIP as it still need a few changes, but it mostly works as
expected (still not fully compliant yet).

So this is based on Lennart's comment in [1]: if the LID state is not reliable,
the kernel should not export the LID switch device as long as we are not sure
about its state.

That is the basic idea, and here are some more general comments:
Lv described the 5 cases in "RFC PATCH v3" regarding the LID switch.
Let me rewrite them here (they are in patch 2):

1. Some platforms send "open" ACPI notification to the OS and the event
   arrive before the button driver is resumed;
2. Some platforms send "open" ACPI notification to the OS, but the event
   arrives after the button driver is resumed, ex., Samsung N210+;
3. Some platforms never send an "open" ACPI notification to the OS, but
   update the cached _LID return value to "open", and this update arrives
   before the button driver is resumed;
4. Some platforms never send an "open" ACPI notification to the OS, but
   update the cached _LID return value to "open", but this update arrives
   after the button driver is resumed, ex., Surface Pro 3;
5. Some platforms never send an "open" ACPI notification to the OS, and
   _LID ACPI method returns a value which stays to "close", ex.,
   Surface Pro 1.

We we consider that we can mark the LID switch as unreliable and make it
disappear when we are not certain of the state, we can consider cases 1, 2, 3
are solved: cases 1 and 3 are solved when the LID state is reliable (majority
of existing laptops), and case 2 is solved just by marking when the LID is not
reliable. When we go to sleep, we unregister the input node. We wait for
the next ACPI notification to re-export the LID switch input node with the
correct state.

Given that the "close" event is reliable, on platforms where the LID switch is
not reliable for "open", we will get the "close" event when we will start
exporting the switch at the input level.

Note that systemd currently doesn't sync the state when the input node just
appears. This is a systemd bug, and it should not be handled by the kernel
community.

For case 4, we are not aware at the acpi/button.c level when the state is valid.
We can solve this by polling every seconds for let's say 1 min, and if we detect
a change, then we can re-export the input node (this hasn't been implemented
yet). After this delay, we can consider the state as valid and export the input
node with the current reported state in the ACPI.

However, this will conflict with case 5 where the ACPI value reported by
the _LID method can be wrong anytime. We will need to treat this separately
or find some other magic to make cases 4 and 5 compatible.

libinput will help cases 4 and 5 to restore the proper state, but that's
assuming we have exported a wrong state. It might happen in case 5, but
shouldn't in case 4.

Anyway, that is just a WIP which IMO is less hacky than the few other series.
I still need to work on the udev/hwdb rules to have the list of problematic
platforms in hwdb to not have them in the kernel, but that shouldn't be much
of an issue. I also need to work on the polling but I'd like to get some inputs
from Lv, Peter and others before spending too much time on it.

Note: yes, there is a lot of boilerplate for the input handler and for the
reliable state, but I think this simplifies the logic as we are all reliying
on the input stack to filter duplicate events.
One other benefit of this boilerplate is that when libinput changes the LID
state, i915 and nouveau will get notified.

Cheers,
Benjamin


[1] https://github.com/systemd/systemd/issues/2807

Benjamin Tissoires (3):
  ACPI: button: extract input creation/destruction helpers
  ACPI: button: remove the LID input node when the state is unknown
  ACPI: button: Let input filter out the LID events

Lv Zheng (1):
  ACPI: button: Fix lid notification locks

 drivers/acpi/button.c | 453 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 320 insertions(+), 133 deletions(-)

-- 
2.9.4