Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753380AbdFSBn7 (ORCPT ); Sun, 18 Jun 2017 21:43:59 -0400 Received: from mga07.intel.com ([134.134.136.100]:7500 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753236AbdFSBn4 (ORCPT ); Sun, 18 Jun 2017 21:43:56 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,359,1493708400"; d="scan'208";a="114697007" From: "Zheng, Lv" To: Bastien Nocera CC: Benjamin Tissoires , 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///tuAAAEOsxgP//haIA//9u+qCAAKDZAP/7QTDA Date: Mon, 19 Jun 2017 01:43:29 +0000 Message-ID: <1AE640813FDE7649BE1B193DEA596E886CED1419@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> <1AE640813FDE7649BE1B193DEA596E886CED0AC1@SHSMSX101.ccr.corp.intel.com> <20170616080950.GM5085@mail.corp.redhat.com> <1AE640813FDE7649BE1B193DEA596E886CED0C5E@SHSMSX101.ccr.corp.intel.com> <33C872C7-EFB2-4CA0-8CAD-8B6E8916180C@hadess.net> In-Reply-To: <33C872C7-EFB2-4CA0-8CAD-8B6E8916180C@hadess.net> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDNlZmEyNjctOGE0YS00NjFlLThkN2QtNjE0MGM3OGMyM2MyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjA3WEoxbkpZTFBGc0ltMWZmV2RxSlM1XC85ZUNTMWdZd1hjeTBnQzV1RWUwPSJ9 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 v5J1i3r2014088 Content-Length: 7498 Lines: 172 Hi, > From: Bastien Nocera [mailto:hadess@hadess.net] > Subject: Re: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI > > > > > On 16 Jun 2017, at 10:53, Zheng, Lv wrote: > > > > Hi, > > > >> 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, Benjamin > >>> > >>> Let me just say something one more time. > >>> > >>>> From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] > >> > >> [snip] > >>>>>>> > >>>>>>> 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? > >> > >> I was using potential to say that some actual devices are sending > >> rightful states, while others are not (we already named them a lot in > >> those countless threads). So potential here is from a user space > >> perspective where you are not sure if the state is reliable or not > >> (given we currently don't have this information about reliability). > >> > >>> 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. > >> > >> No, if there is a delay, there is a false positive, because the initial > >> state is wrong with respect to the device physical state. > >> > >>> > >>>>> 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? > >> > >> Please, you are either trolling or just kidding. If there are delays in > >> the "computing world", these has to be handled by the kernel, and not > >> exported to the user space if the kernel protocol says that the state is > >> reliable. > >> > >>> 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"? > >> > >> No comments, this won't get us anywhere. > >> > >>> > >>> I think most UI programs care no about state stored in the input node, > >>> they only receive events raised from the input node. > >> > >> Bullshit. When you launch such a program, you need to query the state > >> because you won't receive the event that happened way before the launch. > >> > >>> 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? > >> > >> No, all program that listen to LID switches input nodes care about the > >> state. We already told you that, you just don't listen: > >> > >> - systemd cares about the state as it does polling on the input node in > >> case it misses an event > >> - libinput cares about the state as previously mentioned > >> - gnome-setting-daemons cares about the state given it decides whether > >> or not lighting up the monitors depending on the state. And if you > >> relaunch the daemon, it'll query the state to decide what is the best > >> arrangement for the screens > > > > Let's consider this case with delay: > > After resume, gnome-setting-daemon queries SW_LID and got "close". > > Then it lights up the wrong monitors. > > Then I believe "open" will be delivered to it several seconds later. > > Should gnome-setting-daemon light-up correct monitors this time? > > So it just looks like user programs behave with a delay accordingly because of the "platform > turnaround" delay. > > If you implement it in such a way that GNOME settings daemon behaves weirdly, you'll get my revert > request in the mail. Do. Not. Ever. Lie. First, I don't know what should be reverted... I have 2 solutions here for review, and Benjamin has 1. And none of them has been upstreamed. We are just discussing. However we need to get 1 of them upstreamed in next cycle. I think users won't startup gnome-setting-daemon right after resume. It should have already been started. There is only 1 platform may see delayed state update after resume. Let's see if there is a practical issue. 1. Before suspend, the "lid state" is "close", and 2. After resume, the state might remain "close" for a while Since libinput won't deliver close to userspace, and gnome-setting-daemon listens to key switches, there is no wrong behavior. 3. Then after several seconds, "open" arrives. gnome-setting-daemon re-arrange monitors and screen layouts in response to the new event. So there is no problem. IMO, there is no need to improve for post-resume case. Users will just startup gnome-setting-daemon once after boot. And it's likely that when it is started, the state is correct. IMO, there might be a chance to improve for post-boot case using Benjamin's approach. Thanks Lv > > > > >> - KDE should do the same (as it is not a daemon that can catch any > >> events) > > > > Cheers, > > Lv > > ¢éì¹»®&Þ~º&¶¬–+-±éݶ¥Šw®žË›±Êâmébžìbž›­Š{ayºʇڙë,j ­¢f£¢·hš‹àz¹®w¥¢¸ > > ¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾ «‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i