RE: [PATCH 1/2] Revert "ACPI / button: Remove lid_init_state=method mode"

[Zheng, Lv]

Hi, Benjamin

Let's stop endless discussing and focus on our needs.

I just copied my questions here.
You can ask them directly.
For the below inlined replies, I'll stop replying if they are based on dependent on our basic agreements.
And I'll reply if something is really bad from my point of view.

My point of view is:
There is a gap between (BIOS ensured/Windows expected) acpi control method lid device behavior and Linux user space expected acpi control method lid device behavior.
Button driver default behavior should be: button.lid_init_state=ignore
If user space programs have special needs, they can fix problems on their own, via the following mean:
 echo -n "open" > /sys/modules/button/parameters/lid_init_state
 echo -n "close" > /sys/modules/button/parameters/lid_init_state
Keeping open/close modes is because I don't think there is any bug in button driver.
So I need to prepare quirk modes from button driver's point of view and use them as a response to related bug reports reported in acpi community.
Your point of view is:
There is a gap between (BIOS ensured/Windows expected) acpi control method lid device behavior and Linux user space expected acpi control method lid device behavior.
Button driver default behavior should be (not 100% sure if this is your opinion): button.lid_init_state=method
If user space programs have special needs, they can fix them on their own, via the following mean:
 libinput:name:*Lid Switch*:dmi:*svnMicrosoftCorporation:pnSurface3:*
  LIBINPUT_ATTR_LID_SWITCH_RELIABILITY=write_open
From this point of view, we actually don't need open/close modes.

It seems we just need to determine the following first:
1. Who should be responsible for solving bugs triggered by the conflict between bios and linux user space expectations:
   button driver? libinput? Some other user space programs? Users?
2. What should be the default button driver behavior?
   button.lid_init_state=ignore? button.lid_init_state=method?
3. If non button driver quirks are working, button driver quirk modes are useless.
   The question is: Should button.lid_init_state=open/close be kept?
4. From button driver's point of view, button.lid_init_state=ignore seems to be always correct, so we won't abandon it.
   If we can use libinput to manage platform quirks, then button.lid_init_state=method also looks useless.
   The question is: Should button.lid_init_state=method be kept?

I should also let you know my preference:
1. using button.lid_init_state=ignore and button.lid_init_state=method as default behavior is ok for me if answer to 1 is not button driver, otherwise using button.lid_init_state=method is not ok for me
2. deletion of button.lid_init_state=open/close is ok for me if answer to 1 is not button driver, otherwise deletion of button.lid_init_state=open/close is not ok for me
3. deletion of button.lid_init_state=method is always ok for me

See the base line from my side is very clear:
If acpi community need to handle such bug reports, button.lid_init_state=method cannot be the default behavior.
We are just using a different default behavior than "method" to drive things to reach the final root caused solution.

Could you let me know your preference so that we can figure out an agreement between us.
Though I don't know if end users will buy it (they may keep on filing regression reports in ACPI community).

> From: Benjamin Tissoires [mailto:benjamin.tissoires@xxxxxxxxxx]
> Subject: Re: [PATCH 1/2] Revert "ACPI / button: Remove lid_init_state=method mode"

> ...
Skip as it looks the result of debating here can also be concluded from the answers of the questions.
Let's just stop arguing same thing again and again using different contexts.

> > > make reasonable assumptions based on the exact
> > > model capabilities (is the power button accessible with the LID closed),
> >
> > Sounds it's always accessible.
> 
> hmm, I am not sure we have the same fingers.... For me on all the
> laptops I have seen, if the LID is actually (physically) closed, I can
> not press the button. It's a design choice to not have anything powering
> on the laptop when it's in your bag.

Wow...
I can see that recent laptops are having their power buttons on the edge, not on the keypad.
For recent laptop/tablet 2-in-1 PCs or touch laptops which can have their keyboards folded and act notebooks, the power buttons are obviously not on the detachable keypads.
Also for the laptops supporting external monitors, obviously allow users to push power buttons while the lid is closed.

> ...
Skip as it looks the result of debating here can also be concluded from the answers of the questions.


> > > The issue we are fixing here is the fact that the switch state is wrong,
> > > which makes user space assumptions wrong too (and users angry).
> >
> > Considering no platform quirks.
> >
> > If ACPI button driver sends SW_LID, users are likely angry.
> > Unless the user space programs are changed to "ignore open event".
> >
> > If ACPI button driver doesn't send switch events, but key events.
> > The user space programs need to change to handle the new events.
> >
> > So finally whatever we do, user space need to change a bit related to ACPI control method lid device.
> 
> Or we fix the switch to report accurate events/state.
> You do realise that all the energy you are spending, answering to me,
> talking to user space maintainers, users, all comes down to the fact
> that you refuse to have hardware quirks?
> 
> If you don't want to write/maintain the code, fine. I can do it, but
> please stop trying to make everyone else change because you just don't
> want the burden of quirking a handful of laptops.

But I have a very interesting question to ask:
Can button driver/libinput actually fix this without modifying BIOS:

It seems we both rely on user's decision to use proper quirk modes for a specific usage model.
For example, for this case:
 A BIOS never sends lid notification after boot/resume, and _LID return value is close after boot/resume.
Let's see which quirk mode should the user choose before suspending:
1. with no external monitor attached, user should:
    write_open/button.lid_init_state=open
2. with external monitor attached, if user wants the lid to be lit on
    write_open/button.lid_init_state=open
3. with external monitor attached, if user wants the lid not to be lit on
    write_close/button.lid_init_state=close
See there is no single default value for all usage models.

Thus there is no possible __FIX__ for acpi button driver and libinput.
While user space programs can just fix their usage models.

So finally we actually cannot fix anything by maintaining the quirk table, but just create a business of maintaining the quirk table.

> ...
Skip as it looks the result of debating here can also be concluded from the answers of the questions.

> > > > However, is that possible to not introduce platform quirks?
> > > > For example, for systemd:
> > > > If it detected ACPI lid device, automatically switch to an "IgnoreLidOpenEvent" mode (like
> nouveau
> > > drivers' ignorelid=Y option).
> > >
> > > Well, given that 99.9% of laptops have this ACPI lid button, you'll just
> > > remove the feature from all laptops.
> >
> > No, I only removed the wrong usage models related to the strict "open" event.
> > All laptops are still capable of sending correct "close" event.
> 
> My bad, I read too fast and missed the "...Open..." part of
> "IgnoreLidOpenEvent".
> 
> Though I am not sure IgnoreLidOpenEvent is accurate.
> "OpenEventNeverSent" seems to reflect more the reality. But again,
> that's not of our (kernel developers) business.

IMO this is the only root cause fix. :)
It's the only way that the user can use without changing its quirk modes for different usage models.

> > So I already raised this to freedesktop:
> > https://bugs.freedesktop.org/show_bug.cgi?id=100923
> > But couldn't see any one there responding.
> 
> Well, Joachim answered, saying that there is likely a regression in
> acpi/button.c, not in i915.

He surely can add more details as he is responsible of triage of the referenced bug reported on redhat.
But there is no agreement reached yet.

> > > > Can it determine that by its own without listening to the lid key events?
> > >
> > > Basically no. This switch is there for a reason. However, I am convinced
> > > that a good heuristic is to say that if you are using the internal keyboard,
> > > touchscreen or touchpad, unless the user has very very thin fingers, the
> > > lid is open.
> >
> > I'm also convinced that the benefit of having a file in sysfs/procfs to allow user to know if the
> lid is open is marginal.
> 
> I am convinced I don't get your point. We are obviously not talking
> about the same thing here. I was talking about the physical world, where
> the user interact with the laptop...

See my previous reply against accessing power buttons when lid is closed.

> ...
Skip as it looks the result of debating here can also be concluded from the answers of the questions.

> > But it's just 1-2 months Bugzilla silence before seeing a different bug flood trend:
> > This time, they are related to the external monitor usage model:
> > https://bugzilla.kernel.org/show_bug.cgi?id=187271
> > https://bugzilla.redhat.com/show_bug.cgi?id=1430259
> > https://bugzilla.kernel.org/show_bug.cgi?id=195455
> > Now that you can see why I didn't send a patch to change the default behavior to "open" at the time
> we were discussing.
> > That's also why I think we needn't revert back to "method" as the default behavior.
> 
> Still, I strongly disagree. We do not fix 7 devices by breaking
> hundreds. That's just not how the kernel work.

No. The word of "broken" is entirely wrong and emotional here.
If both user space and kernel space are changed to act according to button.lid_init_state=ignore.
No one will be broken.
And no future laptops will be broken.

> ...
Skip as it looks the result of debating here can also be concluded from the answers of the questions.

> > > > After we root cause that's not a kernel problem, do we have mean in other community to handle
> such
> > > reports?
> > > > For example, to collect hwdb entries.
> > >
> > > libinput, systemd are good candidate.
> > > Libinput already has the bits in place, so I'd say we should probably
> > > ask the users to report a bug on the wayland/libinput component of
> > > bugs.freedesktop.org. But this will only work if the default initial lid
> > > state is "method".
> >
> > Good to know. :)
> > I've just changed the category of the forwarded report.
> 
> Well, I am not sure your bug report should have been changed. The bug
> report was regarding i915 being confident in the _LID acpi call, and
> that is not something libinput can change.

OK, I see.

> ...
Skip as it looks the result of debating here can also be concluded from the answers of the questions.

Cheers,
Lv