Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1670209 > unrolled thread

RE: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI

Started by"Zheng, Lv" <lv.zheng@intel.com>
First post2017-06-20 04:50 +0200
Last post2017-06-22 05:20 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  RE: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID  switch exported by ACPI "Zheng, Lv" <lv.zheng@intel.com> - 2017-06-20 04:50 +0200
    Re: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID  switch exported by ACPI Bastien Nocera <hadess@hadess.net> - 2017-06-21 12:30 +0200
      RE: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID  switch exported by ACPI "Zheng, Lv" <lv.zheng@intel.com> - 2017-06-22 05:20 +0200

#1670209 — RE: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-06-20 04:50 +0200
SubjectRE: [systemd-devel] [WIP PATCH 0/4] Rework the unreliable LID switch exported by ACPI
Message-ID<tUhoR-2I0-1@gated-at.bofh.it>
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 Mon, 2017-06-19 at 01:43 +0000, Zheng, Lv wrote:
> > <snip>
> > >
> > > 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.
> 
> The discussion is getting tiring quite frankly. We've been over this
> for nearly a year now, and with no end in sight.

We have concerns to introduce too complicated logics to such a
simple button driver especially the logics are related to platform
firmware, input ABI and user space behaviors.

I understand the situation.
Anyway this shouldn't be a big deal.
Let's prepare a smarter series to collect all fixes and solutions
with runtime configurables and get that to the end users.
So that we can figure out which is the simplest solution.

But before that, let me ask several questions about gnome-setting-deamon.

> 
> > 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.
> 
> It doesn't. It listens to UPower, which tells user-space whether there
> is a lid switch, and whether it's opened or closed.

Thanks for the information.
However I don't see differences here.

> 
> > 3. Then after several seconds, "open" arrives.
> >    gnome-setting-daemon re-arrange monitors and screen layouts in
> > response to the new event.
> 
> Just how is anyone supposed to know that there is an event coming?

Will UPower deliver EV_SW key events to gnome-setting-daemon?

> 
> > 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.
> 
> You cannot rely on when gnome-settings-daemon will be started to make
> *any* decision. Certainly not decisions on how the kernel should
> behave.

My bad wording, I just meant:
When gnome-settings-daemon is started is not related to what we are
discussing.

Do you want to fix regressions?
Or you want to fix new issues on recent platforms?
If you want to fix regressions, I think Benjamin has submitted a revision
to use old method mode, there shouldn't be regressions for
gnome-settings-daemon.

What else we want to do is to fix regressions related to systemd when
we go back to default method mode. Since there is no issue with systemd
233 and after just applying a small change, systemd 229 can also be
worked around, I mean dynamically add/remove input node is not strictly
required for achieving our purposes.

But if you want to fix new issues on new platforms, we can discuss
further and determine which program should be changed and which program
is the best candidate to stop all problems - the ACPI button driver or
the user space.

Cheers
Lv

[toc] | [next] | [standalone]


#1671595

FromBastien Nocera <hadess@hadess.net>
Date2017-06-21 12:30 +0200
Message-ID<tUL3z-4SB-9@gated-at.bofh.it>
In reply to#1670209
On Tue, 2017-06-20 at 02:45 +0000, Zheng, Lv wrote:
> 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 Mon, 2017-06-19 at 01:43 +0000, Zheng, Lv wrote:
> > > <snip>
> > > > 
> > > > 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.
> > 
> > The discussion is getting tiring quite frankly. We've been over
> > this
> > for nearly a year now, and with no end in sight.
> 
> We have concerns to introduce too complicated logics to such a
> simple button driver especially the logics are related to platform
> firmware, input ABI and user space behaviors.
> 
> I understand the situation.
> Anyway this shouldn't be a big deal.
> Let's prepare a smarter series to collect all fixes and solutions
> with runtime configurables and get that to the end users.
> So that we can figure out which is the simplest solution.
> 
> But before that, let me ask several questions about gnome-setting-
> deamon.
> 
> > 
> > > 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.
> > 
> > It doesn't. It listens to UPower, which tells user-space whether
> > there
> > is a lid switch, and whether it's opened or closed.
> 
> Thanks for the information.
> However I don't see differences here.
> 
> > 
> > > 3. Then after several seconds, "open" arrives.
> > >    gnome-setting-daemon re-arrange monitors and screen layouts in
> > > response to the new event.
> > 
> > Just how is anyone supposed to know that there is an event coming?
> 
> Will UPower deliver EV_SW key events to gnome-setting-daemon?
> 
> > 
> > > 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.
> > 
> > You cannot rely on when gnome-settings-daemon will be started to
> > make
> > *any* decision. Certainly not decisions on how the kernel should
> > behave.
> 
> My bad wording, I just meant:
> When gnome-settings-daemon is started is not related to what we are
> discussing.
> 
> Do you want to fix regressions?
> Or you want to fix new issues on recent platforms?
> If you want to fix regressions, I think Benjamin has submitted a
> revision
> to use old method mode, there shouldn't be regressions for
> gnome-settings-daemon.
> 
> What else we want to do is to fix regressions related to systemd when
> we go back to default method mode. Since there is no issue with
> systemd
> 233 and after just applying a small change, systemd 229 can also be
> worked around, I mean dynamically add/remove input node is not
> strictly
> required for achieving our purposes.
> 
> But if you want to fix new issues on new platforms, we can discuss
> further and determine which program should be changed and which
> program
> is the best candidate to stop all problems - the ACPI button driver
> or
> the user space.

I'm happy with Benjamin's patches which don't introduce any
dependencies on new user-space, and don't rely on undocumented
heuristics. What was the API is still the API.

[toc] | [prev] | [next] | [standalone]


#1672274

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-06-22 05:20 +0200
Message-ID<tV0OZ-7gs-15@gated-at.bofh.it>
In reply to#1671595
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 Tue, 2017-06-20 at 02:45 +0000, Zheng, Lv wrote:
> > 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 Mon, 2017-06-19 at 01:43 +0000, Zheng, Lv wrote:
> > > > <snip>
> > > > >
> > > > > 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.
> > >
> > > The discussion is getting tiring quite frankly. We've been over
> > > this
> > > for nearly a year now, and with no end in sight.
> >
> > We have concerns to introduce too complicated logics to such a
> > simple button driver especially the logics are related to platform
> > firmware, input ABI and user space behaviors.
> >
> > I understand the situation.
> > Anyway this shouldn't be a big deal.
> > Let's prepare a smarter series to collect all fixes and solutions
> > with runtime configurables and get that to the end users.
> > So that we can figure out which is the simplest solution.
> >
> > But before that, let me ask several questions about gnome-setting-
> > deamon.
> >
> > >
> > > > 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.
> > >
> > > It doesn't. It listens to UPower, which tells user-space whether
> > > there
> > > is a lid switch, and whether it's opened or closed.
> >
> > Thanks for the information.
> > However I don't see differences here.
> >
> > >
> > > > 3. Then after several seconds, "open" arrives.
> > > >    gnome-setting-daemon re-arrange monitors and screen layouts in
> > > > response to the new event.
> > >
> > > Just how is anyone supposed to know that there is an event coming?
> >
> > Will UPower deliver EV_SW key events to gnome-setting-daemon?
> >
> > >
> > > > 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.
> > >
> > > You cannot rely on when gnome-settings-daemon will be started to
> > > make
> > > *any* decision. Certainly not decisions on how the kernel should
> > > behave.
> >
> > My bad wording, I just meant:
> > When gnome-settings-daemon is started is not related to what we are
> > discussing.
> >
> > Do you want to fix regressions?
> > Or you want to fix new issues on recent platforms?
> > If you want to fix regressions, I think Benjamin has submitted a
> > revision
> > to use old method mode, there shouldn't be regressions for
> > gnome-settings-daemon.
> >
> > What else we want to do is to fix regressions related to systemd when
> > we go back to default method mode. Since there is no issue with
> > systemd
> > 233 and after just applying a small change, systemd 229 can also be
> > worked around, I mean dynamically add/remove input node is not
> > strictly
> > required for achieving our purposes.
> >
> > But if you want to fix new issues on new platforms, we can discuss
> > further and determine which program should be changed and which
> > program
> > is the best candidate to stop all problems - the ACPI button driver
> > or
> > the user space.
> 
> I'm happy with Benjamin's patches which don't introduce any
> dependencies on new user-space, and don't rely on undocumented
> heuristics. What was the API is still the API.

I've put them in the new series as you wish.

But what you say here is not the truth.
With Benjamin's patch, userspace still need to be changed to be able
to know an input node is dynamically created during runtime. So it
is still dependent on new user-space, and even newer than systemd
233.
It will be no difference by just asking:
1. systemd to not to rely on "open" events
   (which has already been fixed in 233).
2. desktop managers to not to rely on LID state to handle layouts.
   (which is what I'm asking to change).
If desktop managers still want to rely on LID state to handle layouts,
kernel drivers still need to be changed to periodically report _LID
state to input layer. And that's beyond what Benjamin's patches can
handle.

Thanks and best regards
Lv

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web