Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752701AbdFPHqE (ORCPT ); Fri, 16 Jun 2017 03:46:04 -0400 Received: from mga11.intel.com ([192.55.52.93]:12062 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752134AbdFPHqC (ORCPT ); Fri, 16 Jun 2017 03:46:02 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,346,1493708400"; d="scan'208";a="115162925" From: "Zheng, Lv" To: Benjamin Tissoires CC: Peter Hutterer , Lennart Poettering , "Wysocki, Rafael J" , "Rafael J . Wysocki" , "Brown, Len" , Lv Zheng , "linux-acpi@vger.kernel.org" , "systemd-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "linux-input@vger.kernel.org" Subject: RE: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI Thread-Topic: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI Thread-Index: AQHS2wd01O/IlIfVsk2vy8j9OHBM16IYiNMAgAmUZoCAAzAyYP//vNyAgACPuZD//4QFgIABmvZA///tuAAAEOsxgA== Date: Fri, 16 Jun 2017 07:45:56 +0000 Message-ID: <1AE640813FDE7649BE1B193DEA596E886CED0AC1@SHSMSX101.ccr.corp.intel.com> References: <20170601184632.2980-1-benjamin.tissoires@redhat.com> <20170607074848.GE27006@gardel-login> <20170613100617.GD29589@mail.corp.redhat.com> <1AE640813FDE7649BE1B193DEA596E886CED0386@SHSMSX101.ccr.corp.intel.com> <20170615064715.GA6195@jelly> <1AE640813FDE7649BE1B193DEA596E886CED047B@SHSMSX101.ccr.corp.intel.com> <20170615075755.GA10001@jelly> <1AE640813FDE7649BE1B193DEA596E886CED0A03@SHSMSX101.ccr.corp.intel.com> <20170616072322.GL5085@mail.corp.redhat.com> In-Reply-To: <20170616072322.GL5085@mail.corp.redhat.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWUzZDNiZDQtZGMxMC00M2JmLWIyNzUtMjkzZGYyYzZkMDI1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Iklsbmc3ZmgrWmE3ZWVmejNQQzlRT3NGb2sydzRiRVpuYXhvOTNjYXNJKzA9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v5G7kEcT016402 Content-Length: 6742 Lines: 144 Hi, Benjamin Let me just say something one more time. > From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] > Subject: Re: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI > > On Jun 16 2017 or thereabouts, Zheng, Lv wrote: > > Hi, > > > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of > Peter > > > Hutterer > > > Subject: Re: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI > > > > > > On Thu, Jun 15, 2017 at 07:33:58AM +0000, Zheng, Lv wrote: > > > > Hi, Peter > > > > > > > > > From: Peter Hutterer [mailto:peter.hutterer@who-t.net] > > > > > Subject: Re: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI > > > > > > > > > > On Thu, Jun 15, 2017 at 02:52:57AM +0000, Zheng, Lv wrote: > > > > > > Hi, Benjamin > > > > > > > > > > > > > From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] > > > > > > > Subject: Re: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by > ACPI > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > [Sorry for the delay, I have been sidetracked from this] > > > > > > > > > > > > > > On Jun 07 2017 or thereabouts, Lennart Poettering wrote: > > > > > > > > On Thu, 01.06.17 20:46, Benjamin Tissoires (benjamin.tissoires@redhat.com) wrote: > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > Ah nice! I (obviously) like this approach. > > > > > > > > > > > > > > Heh. Now I just need to convince Lv that it's the right approach. > > > > > > > > > > > > I feel we don't have big conflicts. > > > > > > And I already took part of your idea into this patchset: > > > > > > https://patchwork.kernel.org/patch/9771121/ > > > > > > https://patchwork.kernel.org/patch/9771119/ > > > > > > I tested my surface pros with Ubuntu, they are working as expected. > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > Uh if this is borked, we should indeed fix this in systemd. Is there > > > > > > > > already a systemd github bug about this? If not, please create one, > > > > > > > > and we'll look into it! > > > > > > > > > > > > > > I don't think there is. I haven't raised it yet because I am not so sure > > > > > > > this will not break again those worthless unreliable LID, and if we play > > > > > > > whack a mole between the kernel and user space, things are going to be > > > > > > > nasty. So I'd rather have this fixed in systemd along with the > > > > > > > unreliable LID switch knowledge, so we are sure that the kernel behaves > > > > > > > the way we expect it to be. > > > > > > > > > > > > This is my feeling: > > > > > > We needn't go that far. > > > > > > We can interpret "input node appears" into "default input node state". > > > > > > > > > > Sorry, can you clarify this bit please? I'm not sure what you mean here. > > > > > Note that there's an unknown amount of time between "device node appearing > > > > > in the system" and when a userspace process actually opens it and looks at > > > > > its state. By then, the node may have changed state again. > > > > > > > > We can see: > > > > "logind" has already implemented a timeout, and will not respond lid state > > > > unless it can be stable within this timeout period. > > > > I'm not an expert of logind, maybe this is because of "HoldOffTimeoutSec"? > > > > > > > > I feel "removing the input node for a period where its state is not trustful" > > > > is technically identical to this mechanism. > > > > > > but you'd be making kernel policy based on one userspace implementation. > > > e.g. libinput doesn't have a timeout period, it assumes the state is > > > correct when an input node is present. > > > > Do you see practical issues? > > Yes, libinput can't rely on the LID switch information to disable > touchpads/touchscreens that are potentially sending false positive. "potential" doesn't mean "practical", right? After applying my last version. There are no false-positives IMO. There are only delays for the reliable key events. ^^^^^^ While the "delay" is very common in computing world. > > After resume, SW_LID state could remain unreliable "close" for a while. > > This is not an option. It is not part of the protocol, having an > unreliable state. > > > But that's just a kind of delay happens in all computing programs. > > I suppose all power managing programs have already handled that. > > I confirmed no breakage for systemd 233. > > For systemd 229, it cannot handle it well due to bugs. > > But my latest patch series has worked the bug around. > > So I don't see any breakage related to post-resume incorrect state period. > > Do you see problems that my tests haven't covered? > > The problems are that you are not following the protocol. And if systemd > 233 works around it, that's good, but systemd is not the only listener > of the LID switch input node, and you are still breaking those by > refusing to follow the specification of the evdev protocol. As you are talking about protocol, let me just ask once. In computing world, 1. delay is very common There are bus turnaround, network turnaround, ... Even measurement itself has delay described by Shannon sampling. Should the delay be a part of the protocol? 2. programs are acting according to rules (we call state machines) States are only determined after measurement (like "quantum states") I have Schroedinger's cat in my mind. Events are determined as they always occur after measurement to trigger "quantum jumps". So for EV_SW protocol, Should programs rely on the reliable "quantum jumps", Or should programs rely on the unreliable "quantum states"? I think most UI programs care no about state stored in the input node, they only receive events raised from the input node. Why should the kernel export a fade-in/fade-out input node to the UI programs and ask them to change? The only program that cares about the state stored in the input node is libinput. So why should every user program be changed to make libinput easier? Cheers, Lv