RE: [PATCH v4 2/2] ACPI / button: Add document for ACPI control method lid device restrictions

From: Zheng, Lv
Date: Sun Jul 24 2016 - 20:39:02 EST

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.
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.