Hi, Bastien > From: Bastien Nocera [mailto:hadess@xxxxxxxxxx] > Subject: Re: [PATCH v4 2/2] ACPI / button: Add document for ACPI control > method lid device restrictions > > On Fri, 2016-07-22 at 11:08 +0200, Benjamin Tissoires wrote: > > > <snip> > > Then you just need to amend the documentation to say that the > > fallback > > of the KEY events is not the "future" but a way to get events on some > > reduced platforms and it will not be the default. > > Please make sure userspace knows that the default is the good SW_LID, > > and some particular cases will need to be handled through the KEY > > events, not the other way around. > > > > [few thoughts later] > > > > How about: > > - you send only one patch with the SW_LID ON/OFF or OFF/ON when we > > receive the notification on buggy platform > > - in the same patch, you add the documentation saying that on most > > platforms, LID is reliable but some don't provide a reliable LID > > state, but you guarantee to send an event when the state changes > > - in userspace, we add the hwdb which says "on this particular > > platform, don't rely on the actual state, but wait for events" -> > > this > > basically removes the polling on these platforms. > > > > Bastien, Dmitry? > > > > I still don't like relying on userspace to actually set the SW_LID > > back to open on resume, as we should not rely on some userspace > > program to set the value (but if logind really wants it, it's up to > > them). > > From my point of view, I would only send the events that can actually > be generated by the system, not any synthetic ones, because user-space > would have no way to know that this was synthetic, and how accurate it > would be. > > So we'd have a separate API, or a separate event for the "close to > Windows behaviour" devices. We'd then use hwdb in udev to tag the > machines that don't have a reliable LID status, in user-space, so we > can have a quick turn around for those machines. > > That should hopefully give us a way to tag test systems, so we can test > the new behaviour, though we'll certainly need to have some changes > made in the stack. [Lv Zheng] That's the original motivation of PATCH 02. However, the PATCH 01 is valid fix. Without it, running SW_LID on such buggy platforms could cause no event. For example, if a platform always reports close, and never reports open. Then after the first SW_LID(close), userspace could never see the follow-up SW_LID(close). Thus that fix is required. Then after upstreaming PATCH 01, we can see something redundant to KEY_LID_XXX approach. Since with PATCH 01, we managed to ensure that platform triggered event will always be delivered to the userspace. Since: 1. Open event is not reliable 2. Close event is reliable We finally can see that: 1. All platform triggered close event can be seen by the userspace as SW_LID(close). 2. On the buggy platforms, SW_LID(open) is meaningless. It then looks like the KEY_LID_XXX is redundant to the improved SW_LID now. As with the key event approach, we still cannot guarantee to send "open" when the state is changed to "opened". __Unless we start to fix the buggy firmware tables__. And what we want to do - delivering reliable "close" to userspace can also be achieved with the SW_LID improvement. Thus, finally, there's no difference between the new userspace behaviors: 1. SW_LID with reliable close: userspace matches hwdb and stops acting upon open 2. KEY_LID_xxx with reliable close: userspace matches hwdb and starts acting only upon KEY_LID_CLOSE So we just need you and Dmitry to reach an agreement here. And this doesn't look like a big conflict. IMO, since SW_LID(CLOSE) is reliable now, we needn't introduce the new KEY_LID_xxx events. That means we can leave the kernel input layer unchanged. And limits this issue to the ACPI subsystem and the userspace programs. What do you think? > > As Benjamin mentioned, it would be nice to have a list of devices that > don't work today, because of this problem. [Lv Zheng] We'll try to find that. Before working out the full list, you can use the above mentioned 3 platforms to test. Cheers ��.n��������+%������w��{.n�����{��)��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�