Hi, Bastien

From: Bastien Nocera [mailto:hadess@hadess.net]
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:

quoted

<snip>

quoted

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