[SRU] [Bionic/Cosmic] [PATCH 0/1] Enable keyboard wakeup for S2Idle laptops

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic/Cosmic] [PATCH 0/1] Enable keyboard wakeup for S2Idle laptops

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1798552

[Impact]
Laptops default to S2Idle can't be woken up by keyboard during suspend.
Most laptops can be woken up by keyboard in S3. To align the behavior,
let's do the same for S2Idle laptops.

[Fix]
Set the serio keyboard wakeup to "enabled" as default.

[Test]
With this patch, XPS 9370 can be woken up by its keyboard during
suspend.

[Regression Potential]
Low. This has the same effect as chaging sysfs knob "power/wakeup" from
"disabled" to "enabled".

[Note]
The commit can be cleanly cherry-picked to both Cosmic and Bionic, but the
formatted patch from Bionic can not be applied to Cosmic and vise versa.
So I created two clean cherry-pickes for both series.

Daniel Drake (1):
  Input: i8042 - enable keyboard wakeups by default when s2idle is used

 drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
 include/linux/suspend.h     |  2 ++
 kernel/power/suspend.c      |  6 ++++++
 3 files changed, 28 insertions(+), 9 deletions(-)

--
2.19.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic] [PATCH 1/1] Input: i8042 - enable keyboard wakeups by default when s2idle is used

Kai-Heng Feng
From: Daniel Drake <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1798552

Previously, on typical consumer laptops, pressing a key on the keyboard
when the system is in suspend would cause it to wake up (default or
unconditional behaviour). This happens because the EC generates a SCI
interrupt in this scenario.

That is no longer true on modern laptops based on Intel WhiskeyLake,
including Acer Swift SF314-55G, Asus UX333FA, Asus UX433FN and Asus
UX533FD. We confirmed with Asus EC engineers that the "Modern Standby"
design has been modified so that the EC no longer generates a SCI
in this case; the keyboard controller itself should be used for wakeup.

In order to retain the standard behaviour of being able to use the
keyboard to wake up the system, enable serio wakeups by default on
platforms that are using s2idle.

Link: https://lkml.kernel.org/r/CAB4CAwfQ0mPMqCLp95TVjw4J0r5zKPWkSvvkK4cpZUGE--w8bQ@...
Reviewed-by: Rafael J. Wysocki <[hidden email]>
Signed-off-by: Daniel Drake <[hidden email]>
Signed-off-by: Dmitry Torokhov <[hidden email]>
(cherry picked from commit 684bec1092b6991ff2a7751e8a763898576eb5c2)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
 include/linux/suspend.h     |  2 ++
 kernel/power/suspend.c      |  6 ++++++
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index a8cd9072481c..7f4592177607 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1392,15 +1392,26 @@ static void __init i8042_register_ports(void)
  for (i = 0; i < I8042_NUM_PORTS; i++) {
  struct serio *serio = i8042_ports[i].serio;
 
- if (serio) {
- printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
- serio->name,
- (unsigned long) I8042_DATA_REG,
- (unsigned long) I8042_COMMAND_REG,
- i8042_ports[i].irq);
- serio_register_port(serio);
- device_set_wakeup_capable(&serio->dev, true);
- }
+ if (!serio)
+ continue;
+
+ printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
+ serio->name,
+ (unsigned long) I8042_DATA_REG,
+ (unsigned long) I8042_COMMAND_REG,
+ i8042_ports[i].irq);
+ serio_register_port(serio);
+ device_set_wakeup_capable(&serio->dev, true);
+
+ /*
+ * On platforms using suspend-to-idle, allow the keyboard to
+ * wake up the system from sleep by enabling keyboard wakeups
+ * by default.  This is consistent with keyboard wakeup
+ * behavior on many platforms using suspend-to-RAM (ACPI S3)
+ * by default.
+ */
+ if (pm_suspend_via_s2idle() && i == I8042_KBD_PORT_NO)
+ device_set_wakeup_enable(&serio->dev, true);
  }
 }
 
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 8544357d92d0..950b56d255ac 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -251,6 +251,7 @@ static inline bool idle_should_enter_s2idle(void)
  return unlikely(s2idle_state == S2IDLE_STATE_ENTER);
 }
 
+extern bool pm_suspend_via_s2idle(void);
 extern void __init pm_states_init(void);
 extern void s2idle_set_ops(const struct platform_s2idle_ops *ops);
 extern void s2idle_wake(void);
@@ -282,6 +283,7 @@ static inline void pm_set_suspend_via_firmware(void) {}
 static inline void pm_set_resume_via_firmware(void) {}
 static inline bool pm_suspend_via_firmware(void) { return false; }
 static inline bool pm_resume_via_firmware(void) { return false; }
+static inline bool pm_suspend_via_s2idle(void) { return false; }
 
 static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {}
 static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 0685c4499431..87e40a7e79e4 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -62,6 +62,12 @@ static DECLARE_WAIT_QUEUE_HEAD(s2idle_wait_head);
 enum s2idle_states __read_mostly s2idle_state;
 static DEFINE_SPINLOCK(s2idle_lock);
 
+bool pm_suspend_via_s2idle(void)
+{
+ return mem_sleep_current == PM_SUSPEND_TO_IDLE;
+}
+EXPORT_SYMBOL_GPL(pm_suspend_via_s2idle);
+
 void s2idle_set_ops(const struct platform_s2idle_ops *ops)
 {
  lock_system_sleep();
--
2.19.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Cosmic] [PATCH 1/1] Input: i8042 - enable keyboard wakeups by default when s2idle is used

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Daniel Drake <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1798552

Previously, on typical consumer laptops, pressing a key on the keyboard
when the system is in suspend would cause it to wake up (default or
unconditional behaviour). This happens because the EC generates a SCI
interrupt in this scenario.

That is no longer true on modern laptops based on Intel WhiskeyLake,
including Acer Swift SF314-55G, Asus UX333FA, Asus UX433FN and Asus
UX533FD. We confirmed with Asus EC engineers that the "Modern Standby"
design has been modified so that the EC no longer generates a SCI
in this case; the keyboard controller itself should be used for wakeup.

In order to retain the standard behaviour of being able to use the
keyboard to wake up the system, enable serio wakeups by default on
platforms that are using s2idle.

Link: https://lkml.kernel.org/r/CAB4CAwfQ0mPMqCLp95TVjw4J0r5zKPWkSvvkK4cpZUGE--w8bQ@...
Reviewed-by: Rafael J. Wysocki <[hidden email]>
Signed-off-by: Daniel Drake <[hidden email]>
Signed-off-by: Dmitry Torokhov <[hidden email]>
(cherry picked from commit 684bec1092b6991ff2a7751e8a763898576eb5c2)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
 include/linux/suspend.h     |  2 ++
 kernel/power/suspend.c      |  6 ++++++
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index a8cd9072481c..7f4592177607 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1392,15 +1392,26 @@ static void __init i8042_register_ports(void)
  for (i = 0; i < I8042_NUM_PORTS; i++) {
  struct serio *serio = i8042_ports[i].serio;
 
- if (serio) {
- printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
- serio->name,
- (unsigned long) I8042_DATA_REG,
- (unsigned long) I8042_COMMAND_REG,
- i8042_ports[i].irq);
- serio_register_port(serio);
- device_set_wakeup_capable(&serio->dev, true);
- }
+ if (!serio)
+ continue;
+
+ printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
+ serio->name,
+ (unsigned long) I8042_DATA_REG,
+ (unsigned long) I8042_COMMAND_REG,
+ i8042_ports[i].irq);
+ serio_register_port(serio);
+ device_set_wakeup_capable(&serio->dev, true);
+
+ /*
+ * On platforms using suspend-to-idle, allow the keyboard to
+ * wake up the system from sleep by enabling keyboard wakeups
+ * by default.  This is consistent with keyboard wakeup
+ * behavior on many platforms using suspend-to-RAM (ACPI S3)
+ * by default.
+ */
+ if (pm_suspend_via_s2idle() && i == I8042_KBD_PORT_NO)
+ device_set_wakeup_enable(&serio->dev, true);
  }
 }
 
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 440b62f7502e..206b735f383f 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -251,6 +251,7 @@ static inline bool idle_should_enter_s2idle(void)
  return unlikely(s2idle_state == S2IDLE_STATE_ENTER);
 }
 
+extern bool pm_suspend_via_s2idle(void);
 extern void __init pm_states_init(void);
 extern void s2idle_set_ops(const struct platform_s2idle_ops *ops);
 extern void s2idle_wake(void);
@@ -282,6 +283,7 @@ static inline void pm_set_suspend_via_firmware(void) {}
 static inline void pm_set_resume_via_firmware(void) {}
 static inline bool pm_suspend_via_firmware(void) { return false; }
 static inline bool pm_resume_via_firmware(void) { return false; }
+static inline bool pm_suspend_via_s2idle(void) { return false; }
 
 static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {}
 static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 87331565e505..3eeafaff190a 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -63,6 +63,12 @@ static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
 enum s2idle_states __read_mostly s2idle_state;
 static DEFINE_RAW_SPINLOCK(s2idle_lock);
 
+bool pm_suspend_via_s2idle(void)
+{
+ return mem_sleep_current == PM_SUSPEND_TO_IDLE;
+}
+EXPORT_SYMBOL_GPL(pm_suspend_via_s2idle);
+
 void s2idle_set_ops(const struct platform_s2idle_ops *ops)
 {
  lock_system_sleep();
--
2.19.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [Bionic/Cosmic] [PATCH 0/1] Enable keyboard wakeup for S2Idle laptops

AceLan Kao
In reply to this post by Kai-Heng Feng
Acked-By: AceLan Kao <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU] [Bionic/Cosmic] [PATCH 0/1] Enable keyboard wakeup for S2Idle laptops

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 18.10.18 11:14, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1798552
>
> [Impact]
> Laptops default to S2Idle can't be woken up by keyboard during suspend.
> Most laptops can be woken up by keyboard in S3. To align the behavior,
> let's do the same for S2Idle laptops.
>
> [Fix]
> Set the serio keyboard wakeup to "enabled" as default.
>
> [Test]
> With this patch, XPS 9370 can be woken up by its keyboard during
> suspend.
>
> [Regression Potential]
> Low. This has the same effect as chaging sysfs knob "power/wakeup" from
> "disabled" to "enabled".
>
> [Note]
> The commit can be cleanly cherry-picked to both Cosmic and Bionic, but the
> formatted patch from Bionic can not be applied to Cosmic and vise versa.
> So I created two clean cherry-pickes for both series.
>
> Daniel Drake (1):
>   Input: i8042 - enable keyboard wakeups by default when s2idle is used
>
>  drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
>  include/linux/suspend.h     |  2 ++
>  kernel/power/suspend.c      |  6 ++++++
>  3 files changed, 28 insertions(+), 9 deletions(-)
>
Is this really a cherry pick for Cosmic and Bionic? If so why submitting twice?

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SRU] [Bionic/Cosmic] [PATCH 0/1] Enable keyboard wakeup for S2Idle laptops

Kai-Heng Feng


> On Nov 6, 2018, at 20:59, Stefan Bader <[hidden email]> wrote:
>
> On 18.10.18 11:14, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1798552
>>
>> [Impact]
>> Laptops default to S2Idle can't be woken up by keyboard during suspend.
>> Most laptops can be woken up by keyboard in S3. To align the behavior,
>> let's do the same for S2Idle laptops.
>>
>> [Fix]
>> Set the serio keyboard wakeup to "enabled" as default.
>>
>> [Test]
>> With this patch, XPS 9370 can be woken up by its keyboard during
>> suspend.
>>
>> [Regression Potential]
>> Low. This has the same effect as chaging sysfs knob "power/wakeup" from
>> "disabled" to "enabled".
>>
>> [Note]
>> The commit can be cleanly cherry-picked to both Cosmic and Bionic, but the
>> formatted patch from Bionic can not be applied to Cosmic and vise versa.
>> So I created two clean cherry-pickes for both series.
>>
>> Daniel Drake (1):
>>  Input: i8042 - enable keyboard wakeups by default when s2idle is used
>>
>> drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
>> include/linux/suspend.h     |  2 ++
>> kernel/power/suspend.c      |  6 ++++++
>> 3 files changed, 28 insertions(+), 9 deletions(-)
>>
> Is this really a cherry pick for Cosmic and Bionic? If so why submitting twice?

I stated the reason in the [Note] section.

I have no idea why `git cherry-pick` works but `git am` formatted patch does not.
That’s why I created two separate clean cherry-picks.

Kai-Heng

>
> -Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [Bionic/Cosmic] [PATCH 0/1] Enable keyboard wakeup for S2Idle laptops

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 18.10.18 11:14, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1798552
>
> [Impact]
> Laptops default to S2Idle can't be woken up by keyboard during suspend.
> Most laptops can be woken up by keyboard in S3. To align the behavior,
> let's do the same for S2Idle laptops.
>
> [Fix]
> Set the serio keyboard wakeup to "enabled" as default.
>
> [Test]
> With this patch, XPS 9370 can be woken up by its keyboard during
> suspend.
>
> [Regression Potential]
> Low. This has the same effect as chaging sysfs knob "power/wakeup" from
> "disabled" to "enabled".
>
> [Note]
> The commit can be cleanly cherry-picked to both Cosmic and Bionic, but the
> formatted patch from Bionic can not be applied to Cosmic and vise versa.
> So I created two clean cherry-pickes for both series.
>
> Daniel Drake (1):
>   Input: i8042 - enable keyboard wakeups by default when s2idle is used
>
>  drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
>  include/linux/suspend.h     |  2 ++
>  kernel/power/suspend.c      |  6 ++++++
>  3 files changed, 28 insertions(+), 9 deletions(-)
>
Ok then, thanks.

Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU] [Bionic/Cosmic] [PATCH 0/1] Enable keyboard wakeup for S2Idle laptops

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 18.10.18 11:14, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1798552
>
> [Impact]
> Laptops default to S2Idle can't be woken up by keyboard during suspend.
> Most laptops can be woken up by keyboard in S3. To align the behavior,
> let's do the same for S2Idle laptops.
>
> [Fix]
> Set the serio keyboard wakeup to "enabled" as default.
>
> [Test]
> With this patch, XPS 9370 can be woken up by its keyboard during
> suspend.
>
> [Regression Potential]
> Low. This has the same effect as chaging sysfs knob "power/wakeup" from
> "disabled" to "enabled".
>
> [Note]
> The commit can be cleanly cherry-picked to both Cosmic and Bionic, but the
> formatted patch from Bionic can not be applied to Cosmic and vise versa.
> So I created two clean cherry-pickes for both series.
>
> Daniel Drake (1):
>   Input: i8042 - enable keyboard wakeups by default when s2idle is used
>
>  drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
>  include/linux/suspend.h     |  2 ++
>  kernel/power/suspend.c      |  6 ++++++
>  3 files changed, 28 insertions(+), 9 deletions(-)
>
Applied to bionic(1),cosmic /master-next. Thanks.

-Stefan

(1) also rebased bionic so this patch is applied before those for
    https://bugs.launchpad.net/buga/1801875


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment