* [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle`
@ 2022-07-01 2:33 Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 02/10] ACPI / x86: Use new `pm_suspend_preferred_s2idle` Mario Limonciello
` (9 more replies)
0 siblings, 10 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, Rafael J. Wysocki, Pavel Machek, Len Brown
Cc: linux-pm, linux-kernel
Many drivers in the kernel will check the FADT to determine if low
power idle is supported and use this information to set up a policy
decision in the driver. To abstract this information from drivers
introduce a new helper symbol that can indicate this information.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
include/linux/suspend.h | 1 +
kernel/power/suspend.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 70f2921e2e70..9d911e026720 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -305,6 +305,7 @@ static inline bool idle_should_enter_s2idle(void)
return unlikely(s2idle_state == S2IDLE_STATE_ENTER);
}
+extern bool pm_suspend_preferred_s2idle(void);
extern bool pm_suspend_default_s2idle(void);
extern void __init pm_states_init(void);
extern void s2idle_set_ops(const struct platform_s2idle_ops *ops);
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 827075944d28..0030e7dfe6cf 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -9,6 +9,7 @@
#define pr_fmt(fmt) "PM: " fmt
+#include <linux/acpi.h>
#include <linux/string.h>
#include <linux/delay.h>
#include <linux/errno.h>
@@ -61,6 +62,22 @@ static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
enum s2idle_states __read_mostly s2idle_state;
static DEFINE_RAW_SPINLOCK(s2idle_lock);
+/**
+ * pm_suspend_preferred_s2idle - Check if suspend-to-idle is the preferred suspend method.
+ *
+ * Return 'true' if suspend-to-idle is preferred by the system designer for the default
+ * suspend method.
+ */
+bool pm_suspend_preferred_s2idle(void)
+{
+#ifdef CONFIG_ACPI
+ return acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0;
+#else
+ return false;
+#endif
+}
+EXPORT_SYMBOL_GPL(pm_suspend_preferred_s2idle);
+
/**
* pm_suspend_default_s2idle - Check if suspend-to-idle is the default suspend.
*
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 02/10] ACPI / x86: Use new `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-13 18:33 ` Rafael J. Wysocki
2022-07-01 2:33 ` [PATCH v3 03/10] ACPI: LPIT: " Mario Limonciello
` (8 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, linux-kernel
Cc: linux-pm, Rafael J. Wysocki, Len Brown, linux-acpi
Drop the direct check from the FADT and use the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/acpi/x86/s2idle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 2963229062f8..b1483d5092c1 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -369,7 +369,7 @@ static int lps0_device_attach(struct acpi_device *adev,
if (lps0_device_handle)
return 0;
- if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+ if (!pm_suspend_preferred_s2idle())
return 0;
if (acpi_s2idle_vendor_amd()) {
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 03/10] ACPI: LPIT: Use new `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 02/10] ACPI / x86: Use new `pm_suspend_preferred_s2idle` Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 13:01 ` kernel test robot
2022-07-01 2:33 ` [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle` Mario Limonciello
` (7 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, Rafael J. Wysocki, Len Brown
Cc: linux-pm, linux-acpi, linux-kernel
Drop the direct check from the FADT and use the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/acpi/acpi_lpit.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/acpi_lpit.c b/drivers/acpi/acpi_lpit.c
index 48e5059d67ca..31c49107c579 100644
--- a/drivers/acpi/acpi_lpit.c
+++ b/drivers/acpi/acpi_lpit.c
@@ -6,6 +6,7 @@
* Copyright (C) 2017 Intel Corporation. All rights reserved.
*/
+#include <linux/suspend.h>
#include <linux/cpu.h>
#include <linux/acpi.h>
#include <asm/msr.h>
@@ -109,7 +110,7 @@ static void lpit_update_residency(struct lpit_residency_info *info,
if (!info->iomem_addr)
return;
- if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+ if (!pm_suspend_preferred_s2idle())
return;
/* Silently fail, if cpuidle attribute group is not present */
@@ -117,7 +118,7 @@ static void lpit_update_residency(struct lpit_residency_info *info,
&dev_attr_low_power_idle_system_residency_us.attr,
"cpuidle");
} else if (info->gaddr.space_id == ACPI_ADR_SPACE_FIXED_HARDWARE) {
- if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+ if (!pm_suspend_preferred_s2idle())
return;
/* Silently fail, if cpuidle attribute group is not present */
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 02/10] ACPI / x86: Use new `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 03/10] ACPI: LPIT: " Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 5:06 ` Damien Le Moal
2022-07-02 9:34 ` kernel test robot
2022-07-01 2:33 ` [PATCH v3 05/10] rtc: cmos: " Mario Limonciello
` (6 subsequent siblings)
9 siblings, 2 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, Damien Le Moal; +Cc: linux-pm, linux-ide, linux-kernel
Drop the direct check from the FADT and use the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/ata/ahci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index c1eca72b4575..3f79b732dd00 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1611,7 +1611,7 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap,
#ifdef CONFIG_ACPI
if (policy > ATA_LPM_MED_POWER &&
- (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) {
+ pm_suspend_preferred_s2idle()) {
if (hpriv->cap & HOST_CAP_PART)
policy = ATA_LPM_MIN_POWER_WITH_PARTIAL;
else if (hpriv->cap & HOST_CAP_SSC)
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 05/10] rtc: cmos: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
` (2 preceding siblings ...)
2022-07-01 2:33 ` [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle` Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 11:50 ` kernel test robot
2022-07-01 2:33 ` [PATCH v3 06/10] thermal: intel: pch: " Mario Limonciello
` (5 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, Alessandro Zummo, Alexandre Belloni
Cc: linux-pm, linux-rtc, linux-kernel
Drop the direct check from the FADT and use the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/rtc/rtc-cmos.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 7c006c2b125f..2e3d35e87061 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -32,6 +32,7 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/spinlock.h>
+#include <linux/suspend.h>
#include <linux/platform_device.h>
#include <linux/log2.h>
#include <linux/pm.h>
@@ -1260,7 +1261,7 @@ static void use_acpi_alarm_quirks(void)
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
return;
- if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+ if (!pm_suspend_preferred_s2idle())
return;
if (!is_hpet_enabled())
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 06/10] thermal: intel: pch: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
` (3 preceding siblings ...)
2022-07-01 2:33 ` [PATCH v3 05/10] rtc: cmos: " Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 07/10] drm/amd: " Mario Limonciello
` (4 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, linux-kernel
Cc: linux-pm, Rafael J. Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui
Drop the direct check from the FADT and use the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/thermal/intel/intel_pch_thermal.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
index c1fa2b29b153..3124c4b1da76 100644
--- a/drivers/thermal/intel/intel_pch_thermal.c
+++ b/drivers/thermal/intel/intel_pch_thermal.c
@@ -208,12 +208,8 @@ static int pch_wpt_suspend(struct pch_thermal_device *ptd)
}
/* Do not check temperature if it is not a S0ix capable platform */
-#ifdef CONFIG_ACPI
- if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+ if (!pm_suspend_preferred_s2idle())
return 0;
-#else
- return 0;
-#endif
/* Do not check temperature if it is not s2idle */
if (pm_suspend_via_firmware())
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 07/10] drm/amd: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
` (4 preceding siblings ...)
2022-07-01 2:33 ` [PATCH v3 06/10] thermal: intel: pch: " Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 08/10] drm/amd: Use `pm_suspend_default_s2idle` Mario Limonciello
` (3 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, linux-kernel
Cc: linux-pm, Alex Deucher, Christian König, Pan, Xinhui,
David Airlie, Daniel Vetter, amd-gfx, dri-devel
Drop the direct check from the FADT and use the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 98ac53ee6bb5..2146232c62ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1072,7 +1072,7 @@ bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
(pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
return false;
- if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) {
+ if (!pm_suspend_preferred_s2idle()) {
dev_warn_once(adev->dev,
"Power consumption will be higher as BIOS has not been configured for suspend-to-idle.\n"
"To use suspend-to-idle change the sleep mode in BIOS setup.\n");
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 08/10] drm/amd: Use `pm_suspend_default_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
` (5 preceding siblings ...)
2022-07-01 2:33 ` [PATCH v3 07/10] drm/amd: " Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 09/10] HID: I2C-HID: Use `pm_suspend_preferred_s2idle` Mario Limonciello
` (2 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, linux-kernel
Cc: linux-pm, Alex Deucher, Christian König, Pan, Xinhui,
David Airlie, Daniel Vetter, amd-gfx, dri-devel
Rather than examining the suspend target, examine what the system is
configured to use. This should be no functional change, just improves
readability by taking the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 2146232c62ef..fc2c6e311979 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1056,7 +1056,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
{
if (adev->flags & AMD_IS_APU)
return false;
- return pm_suspend_target_state != PM_SUSPEND_TO_IDLE;
+ return !pm_suspend_default_s2idle();
}
/**
@@ -1069,7 +1069,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
{
if (!(adev->flags & AMD_IS_APU) ||
- (pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
+ !pm_suspend_default_s2idle())
return false;
if (!pm_suspend_preferred_s2idle()) {
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 09/10] HID: I2C-HID: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
` (6 preceding siblings ...)
2022-07-01 2:33 ` [PATCH v3 08/10] drm/amd: Use `pm_suspend_default_s2idle` Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources Mario Limonciello
2022-07-12 19:06 ` [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Rafael J. Wysocki
9 siblings, 0 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, linux-kernel
Cc: linux-pm, Jiri Kosina, Benjamin Tissoires, linux-input
Drop the direct check from the FADT and use the helper instead.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/hid/i2c-hid/i2c-hid-acpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index b96ae15e0ad9..84d51f309c53 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -105,7 +105,7 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
acpi_device_fix_up_power(adev);
- if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) {
+ if (pm_suspend_preferred_s2idle()) {
device_set_wakeup_capable(dev, true);
device_set_wakeup_enable(dev, false);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
` (7 preceding siblings ...)
2022-07-01 2:33 ` [PATCH v3 09/10] HID: I2C-HID: Use `pm_suspend_preferred_s2idle` Mario Limonciello
@ 2022-07-01 2:33 ` Mario Limonciello
2022-07-01 6:57 ` Greg KH
` (2 more replies)
2022-07-12 19:06 ` [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Rafael J. Wysocki
9 siblings, 3 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 2:33 UTC (permalink / raw)
To: mario.limonciello, Jiri Kosina, Benjamin Tissoires
Cc: linux-pm, Richard Gong, linux-usb, linux-input, linux-kernel
The USB HID transport layer doesn't configure mice for wakeup by default.
Thus users can not wake system from s2idle via USB mouse. However, users
can wake the same system from Modern Standby on Windows with the same USB
mouse.
Microsoft documentation indicates that all USB mice and touchpads should
be waking the system from Modern Standby.
Many people who have used Windows on a PC that supports Modern Standby
have an expectation that s2idle wakeup sources should behave the same in
Linux. For example if your PC is configured "dual-boot" and is used docked
it's very common to wakeup by using a USB mouse connected to your dock in
Windows. Switching to Linux this is not enabled by default and you'll
need to manually turn it on or use a different wakeup source than you did
for Windows.
Changes for wakeups have been made in other subsystems such as the PS/2
keyboard driver which align how wakeup sources in Linux and Modern Standby
in Windows behave. To align expectations from users on USB mice, make this
behavior the same when the system is configured both by the OEM and the
user to use s2idle in Linux.
This means that at a minimum supported mice will be able to wakeup by
clicking a button. If the USB mouse is powered over the s2idle cycle (such
as a wireless mouse with a battery) it's also possible that moving it
may wake up the system. This is HW dependent behavior.
If the user sets the system to use S3 instead of s2idle, or the OEM ships
the system defaulting to S3, this behavior will not be turned on by
default.
Users who have a modern laptop that supports s2idle and use s2idle but
prefer the previous Linux kernel behavior can turn this off via a udev
rule.
Link: https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-wake-sources#input-devices-1
Link: https://lore.kernel.org/linux-usb/20220404214557.3329796-1-richard.gong@amd.com/
Suggested-by: Richard Gong <richard.gong@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
More people keep coming to us confused that they couldn't wake a Linux system
up from sleep using a mouse, so this patch is being revived.
Microsoft documentation doesn't indicate any allowlist for this behavior, and
they actually prescribe it for all USB mice and touchpads.
changes from v2->v3:
* Use `pm_suspend_preferred_s2idle`
* Drop now unnecessary acpi.h header inclusion
* Update commit message
* Adjust comments from v2 per thread
changes from v1->v2:
* Resubmit by Mario
* Update commit message
* Only activate on systems configured by user and OEM for using s2idle
---
drivers/hid/usbhid/hid-core.c | 37 ++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 4490e2f7252a..d08511f00d3b 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -26,6 +26,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/string.h>
+#include <linux/suspend.h>
#include <linux/usb.h>
@@ -1176,17 +1177,31 @@ static int usbhid_start(struct hid_device *hid)
usb_autopm_put_interface(usbhid->intf);
}
- /* Some keyboards don't work until their LEDs have been set.
- * Since BIOSes do set the LEDs, it must be safe for any device
- * that supports the keyboard boot protocol.
- * In addition, enable remote wakeup by default for all keyboard
- * devices supporting the boot protocol.
- */
- if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT &&
- interface->desc.bInterfaceProtocol ==
- USB_INTERFACE_PROTOCOL_KEYBOARD) {
- usbhid_set_leds(hid);
- device_set_wakeup_enable(&dev->dev, 1);
+ if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT) {
+ switch (interface->desc.bInterfaceProtocol) {
+ /* Some keyboards don't work until their LEDs have been set.
+ * Since BIOSes do set the LEDs, it must be safe for any device
+ * that supports the keyboard boot protocol.
+ * In addition, enable remote wakeup by default for all keyboard
+ * devices supporting the boot protocol.
+ */
+ case USB_INTERFACE_PROTOCOL_KEYBOARD:
+ usbhid_set_leds(hid);
+ device_set_wakeup_enable(&dev->dev, 1);
+ break;
+ /*
+ * Windows configures USB mice to be a wakeup source from Modern
+ * Standby, and users have expectations that s2idle wakeup sources
+ * behave the same. Thus setup remote wakeup by default for mice
+ * supporting boot protocol if the system supports s2idle and the user
+ * has not disabled it on the kernel command line.
+ */
+ case USB_INTERFACE_PROTOCOL_MOUSE:
+ if (pm_suspend_preferred_s2idle() &&
+ pm_suspend_default_s2idle())
+ device_set_wakeup_enable(&dev->dev, 1);
+ break;
+ }
}
mutex_unlock(&usbhid->mutex);
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 ` [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle` Mario Limonciello
@ 2022-07-01 5:06 ` Damien Le Moal
2022-07-01 18:01 ` Mario Limonciello
2022-07-02 9:34 ` kernel test robot
1 sibling, 1 reply; 23+ messages in thread
From: Damien Le Moal @ 2022-07-01 5:06 UTC (permalink / raw)
To: Mario Limonciello; +Cc: linux-pm, linux-ide, linux-kernel
On 7/1/22 11:33, Mario Limonciello wrote:
> Drop the direct check from the FADT and use the helper instead.
Where is this helper defined ? Seeing that this is patch 4/10, I have no
context for this patch. Please send the whole series next time.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/ata/ahci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index c1eca72b4575..3f79b732dd00 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1611,7 +1611,7 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap,
>
> #ifdef CONFIG_ACPI
> if (policy > ATA_LPM_MED_POWER &&
> - (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) {
> + pm_suspend_preferred_s2idle()) {
> if (hpriv->cap & HOST_CAP_PART)
> policy = ATA_LPM_MIN_POWER_WITH_PARTIAL;
> else if (hpriv->cap & HOST_CAP_SSC)
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources
2022-07-01 2:33 ` [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources Mario Limonciello
@ 2022-07-01 6:57 ` Greg KH
2022-07-01 6:58 ` Greg KH
2022-07-01 11:30 ` kernel test robot
2 siblings, 0 replies; 23+ messages in thread
From: Greg KH @ 2022-07-01 6:57 UTC (permalink / raw)
To: Mario Limonciello
Cc: Jiri Kosina, Benjamin Tissoires, linux-pm, Richard Gong,
linux-usb, linux-input, linux-kernel
On Thu, Jun 30, 2022 at 09:33:28PM -0500, Mario Limonciello wrote:
> The USB HID transport layer doesn't configure mice for wakeup by default.
> Thus users can not wake system from s2idle via USB mouse. However, users
> can wake the same system from Modern Standby on Windows with the same USB
> mouse.
>
> Microsoft documentation indicates that all USB mice and touchpads should
> be waking the system from Modern Standby.
>
> Many people who have used Windows on a PC that supports Modern Standby
> have an expectation that s2idle wakeup sources should behave the same in
> Linux. For example if your PC is configured "dual-boot" and is used docked
> it's very common to wakeup by using a USB mouse connected to your dock in
> Windows. Switching to Linux this is not enabled by default and you'll
> need to manually turn it on or use a different wakeup source than you did
> for Windows.
>
> Changes for wakeups have been made in other subsystems such as the PS/2
> keyboard driver which align how wakeup sources in Linux and Modern Standby
> in Windows behave. To align expectations from users on USB mice, make this
> behavior the same when the system is configured both by the OEM and the
> user to use s2idle in Linux.
>
> This means that at a minimum supported mice will be able to wakeup by
> clicking a button. If the USB mouse is powered over the s2idle cycle (such
> as a wireless mouse with a battery) it's also possible that moving it
> may wake up the system. This is HW dependent behavior.
>
> If the user sets the system to use S3 instead of s2idle, or the OEM ships
> the system defaulting to S3, this behavior will not be turned on by
> default.
>
> Users who have a modern laptop that supports s2idle and use s2idle but
> prefer the previous Linux kernel behavior can turn this off via a udev
> rule.
>
> Link: https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-wake-sources#input-devices-1
> Link: https://lore.kernel.org/linux-usb/20220404214557.3329796-1-richard.gong@amd.com/
> Suggested-by: Richard Gong <richard.gong@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> More people keep coming to us confused that they couldn't wake a Linux system
> up from sleep using a mouse, so this patch is being revived.
>
> Microsoft documentation doesn't indicate any allowlist for this behavior, and
> they actually prescribe it for all USB mice and touchpads.
Note, this is very different than older versions of Windows. Any idea
when this behavior and prescription changed?
Also, how are you going to handle the differences between "internal" and
"external" USB mice and touchpads? The microsoft link above only
references external USB and Bluetooth devices, not internal ones.
>
> changes from v2->v3:
> * Use `pm_suspend_preferred_s2idle`
> * Drop now unnecessary acpi.h header inclusion
> * Update commit message
> * Adjust comments from v2 per thread
>
> changes from v1->v2:
> * Resubmit by Mario
> * Update commit message
> * Only activate on systems configured by user and OEM for using s2idle
> ---
> drivers/hid/usbhid/hid-core.c | 37 ++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
Where are patches 1-9 of this series?
confused,
greg k-h
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources
2022-07-01 2:33 ` [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources Mario Limonciello
2022-07-01 6:57 ` Greg KH
@ 2022-07-01 6:58 ` Greg KH
2022-07-01 18:00 ` Mario Limonciello
2022-07-01 11:30 ` kernel test robot
2 siblings, 1 reply; 23+ messages in thread
From: Greg KH @ 2022-07-01 6:58 UTC (permalink / raw)
To: Mario Limonciello
Cc: Jiri Kosina, Benjamin Tissoires, linux-pm, Richard Gong,
linux-usb, linux-input, linux-kernel
On Thu, Jun 30, 2022 at 09:33:28PM -0500, Mario Limonciello wrote:
> The USB HID transport layer doesn't configure mice for wakeup by default.
> Thus users can not wake system from s2idle via USB mouse. However, users
> can wake the same system from Modern Standby on Windows with the same USB
> mouse.
>
> Microsoft documentation indicates that all USB mice and touchpads should
> be waking the system from Modern Standby.
As I said before, their documentation indicates that all _EXTERNAL_ mice
and touchpads. You forgot about internally connected mice and touchpads
here, you wouldn't want them to wake up a device asleep, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources
2022-07-01 2:33 ` [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources Mario Limonciello
2022-07-01 6:57 ` Greg KH
2022-07-01 6:58 ` Greg KH
@ 2022-07-01 11:30 ` kernel test robot
2 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2022-07-01 11:30 UTC (permalink / raw)
To: Mario Limonciello, Jiri Kosina, Benjamin Tissoires
Cc: llvm, kbuild-all, linux-pm, Richard Gong, linux-usb, linux-input,
linux-kernel
Hi Mario,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on drm-misc/drm-misc-next hid/for-next linus/master v5.19-rc4 next-20220701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220701/202207011931.a4oqLMtN-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/54a1cce9cd825e0570d307b44a695f04bba77fd2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
git checkout 54a1cce9cd825e0570d307b44a695f04bba77fd2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hid/usbhid/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/hid/usbhid/hid-core.c:1200:8: error: call to undeclared function 'pm_suspend_preferred_s2idle'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (pm_suspend_preferred_s2idle() &&
^
drivers/hid/usbhid/hid-core.c:1200:8: note: did you mean 'pm_suspend_default_s2idle'?
include/linux/suspend.h:343:20: note: 'pm_suspend_default_s2idle' declared here
static inline bool pm_suspend_default_s2idle(void) { return false; }
^
1 error generated.
vim +/pm_suspend_preferred_s2idle +1200 drivers/hid/usbhid/hid-core.c
1060
1061 static int usbhid_start(struct hid_device *hid)
1062 {
1063 struct usb_interface *intf = to_usb_interface(hid->dev.parent);
1064 struct usb_host_interface *interface = intf->cur_altsetting;
1065 struct usb_device *dev = interface_to_usbdev(intf);
1066 struct usbhid_device *usbhid = hid->driver_data;
1067 unsigned int n, insize = 0;
1068 int ret;
1069
1070 mutex_lock(&usbhid->mutex);
1071
1072 clear_bit(HID_DISCONNECTED, &usbhid->iofl);
1073
1074 usbhid->bufsize = HID_MIN_BUFFER_SIZE;
1075 hid_find_max_report(hid, HID_INPUT_REPORT, &usbhid->bufsize);
1076 hid_find_max_report(hid, HID_OUTPUT_REPORT, &usbhid->bufsize);
1077 hid_find_max_report(hid, HID_FEATURE_REPORT, &usbhid->bufsize);
1078
1079 if (usbhid->bufsize > HID_MAX_BUFFER_SIZE)
1080 usbhid->bufsize = HID_MAX_BUFFER_SIZE;
1081
1082 hid_find_max_report(hid, HID_INPUT_REPORT, &insize);
1083
1084 if (insize > HID_MAX_BUFFER_SIZE)
1085 insize = HID_MAX_BUFFER_SIZE;
1086
1087 if (hid_alloc_buffers(dev, hid)) {
1088 ret = -ENOMEM;
1089 goto fail;
1090 }
1091
1092 for (n = 0; n < interface->desc.bNumEndpoints; n++) {
1093 struct usb_endpoint_descriptor *endpoint;
1094 int pipe;
1095 int interval;
1096
1097 endpoint = &interface->endpoint[n].desc;
1098 if (!usb_endpoint_xfer_int(endpoint))
1099 continue;
1100
1101 interval = endpoint->bInterval;
1102
1103 /* Some vendors give fullspeed interval on highspeed devides */
1104 if (hid->quirks & HID_QUIRK_FULLSPEED_INTERVAL &&
1105 dev->speed == USB_SPEED_HIGH) {
1106 interval = fls(endpoint->bInterval*8);
1107 pr_info("%s: Fixing fullspeed to highspeed interval: %d -> %d\n",
1108 hid->name, endpoint->bInterval, interval);
1109 }
1110
1111 /* Change the polling interval of mice, joysticks
1112 * and keyboards.
1113 */
1114 switch (hid->collection->usage) {
1115 case HID_GD_MOUSE:
1116 if (hid_mousepoll_interval > 0)
1117 interval = hid_mousepoll_interval;
1118 break;
1119 case HID_GD_JOYSTICK:
1120 if (hid_jspoll_interval > 0)
1121 interval = hid_jspoll_interval;
1122 break;
1123 case HID_GD_KEYBOARD:
1124 if (hid_kbpoll_interval > 0)
1125 interval = hid_kbpoll_interval;
1126 break;
1127 }
1128
1129 ret = -ENOMEM;
1130 if (usb_endpoint_dir_in(endpoint)) {
1131 if (usbhid->urbin)
1132 continue;
1133 if (!(usbhid->urbin = usb_alloc_urb(0, GFP_KERNEL)))
1134 goto fail;
1135 pipe = usb_rcvintpipe(dev, endpoint->bEndpointAddress);
1136 usb_fill_int_urb(usbhid->urbin, dev, pipe, usbhid->inbuf, insize,
1137 hid_irq_in, hid, interval);
1138 usbhid->urbin->transfer_dma = usbhid->inbuf_dma;
1139 usbhid->urbin->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
1140 } else {
1141 if (usbhid->urbout)
1142 continue;
1143 if (!(usbhid->urbout = usb_alloc_urb(0, GFP_KERNEL)))
1144 goto fail;
1145 pipe = usb_sndintpipe(dev, endpoint->bEndpointAddress);
1146 usb_fill_int_urb(usbhid->urbout, dev, pipe, usbhid->outbuf, 0,
1147 hid_irq_out, hid, interval);
1148 usbhid->urbout->transfer_dma = usbhid->outbuf_dma;
1149 usbhid->urbout->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
1150 }
1151 }
1152
1153 usbhid->urbctrl = usb_alloc_urb(0, GFP_KERNEL);
1154 if (!usbhid->urbctrl) {
1155 ret = -ENOMEM;
1156 goto fail;
1157 }
1158
1159 usb_fill_control_urb(usbhid->urbctrl, dev, 0, (void *) usbhid->cr,
1160 usbhid->ctrlbuf, 1, hid_ctrl, hid);
1161 usbhid->urbctrl->transfer_dma = usbhid->ctrlbuf_dma;
1162 usbhid->urbctrl->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
1163
1164 set_bit(HID_STARTED, &usbhid->iofl);
1165
1166 if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
1167 ret = usb_autopm_get_interface(usbhid->intf);
1168 if (ret)
1169 goto fail;
1170 set_bit(HID_IN_POLLING, &usbhid->iofl);
1171 usbhid->intf->needs_remote_wakeup = 1;
1172 ret = hid_start_in(hid);
1173 if (ret) {
1174 dev_err(&hid->dev,
1175 "failed to start in urb: %d\n", ret);
1176 }
1177 usb_autopm_put_interface(usbhid->intf);
1178 }
1179
1180 if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT) {
1181 switch (interface->desc.bInterfaceProtocol) {
1182 /* Some keyboards don't work until their LEDs have been set.
1183 * Since BIOSes do set the LEDs, it must be safe for any device
1184 * that supports the keyboard boot protocol.
1185 * In addition, enable remote wakeup by default for all keyboard
1186 * devices supporting the boot protocol.
1187 */
1188 case USB_INTERFACE_PROTOCOL_KEYBOARD:
1189 usbhid_set_leds(hid);
1190 device_set_wakeup_enable(&dev->dev, 1);
1191 break;
1192 /*
1193 * Windows configures USB mice to be a wakeup source from Modern
1194 * Standby, and users have expectations that s2idle wakeup sources
1195 * behave the same. Thus setup remote wakeup by default for mice
1196 * supporting boot protocol if the system supports s2idle and the user
1197 * has not disabled it on the kernel command line.
1198 */
1199 case USB_INTERFACE_PROTOCOL_MOUSE:
> 1200 if (pm_suspend_preferred_s2idle() &&
1201 pm_suspend_default_s2idle())
1202 device_set_wakeup_enable(&dev->dev, 1);
1203 break;
1204 }
1205 }
1206
1207 mutex_unlock(&usbhid->mutex);
1208 return 0;
1209
1210 fail:
1211 usb_free_urb(usbhid->urbin);
1212 usb_free_urb(usbhid->urbout);
1213 usb_free_urb(usbhid->urbctrl);
1214 usbhid->urbin = NULL;
1215 usbhid->urbout = NULL;
1216 usbhid->urbctrl = NULL;
1217 hid_free_buffers(dev, hid);
1218 mutex_unlock(&usbhid->mutex);
1219 return ret;
1220 }
1221
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 05/10] rtc: cmos: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 ` [PATCH v3 05/10] rtc: cmos: " Mario Limonciello
@ 2022-07-01 11:50 ` kernel test robot
0 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2022-07-01 11:50 UTC (permalink / raw)
To: Mario Limonciello, Alessandro Zummo, Alexandre Belloni
Cc: kbuild-all, linux-pm, linux-rtc, linux-kernel
Hi Mario,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on drm-misc/drm-misc-next hid/for-next linus/master v5.19-rc4 next-20220701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20220701/202207011940.s0fsRy23-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/f5c9ad12802ecfaf2fc73e35ee9b563d7283b049
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
git checkout f5c9ad12802ecfaf2fc73e35ee9b563d7283b049
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/rtc/rtc-cmos.c: In function 'use_acpi_alarm_quirks':
>> drivers/rtc/rtc-cmos.c:1264:14: error: implicit declaration of function 'pm_suspend_preferred_s2idle'; did you mean 'pm_suspend_default_s2idle'? [-Werror=implicit-function-declaration]
1264 | if (!pm_suspend_preferred_s2idle())
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
| pm_suspend_default_s2idle
cc1: some warnings being treated as errors
vim +1264 drivers/rtc/rtc-cmos.c
1256
1257 #ifdef CONFIG_X86
1258 /* Enable use_acpi_alarm mode for Intel platforms no earlier than 2015 */
1259 static void use_acpi_alarm_quirks(void)
1260 {
1261 if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
1262 return;
1263
> 1264 if (!pm_suspend_preferred_s2idle())
1265 return;
1266
1267 if (!is_hpet_enabled())
1268 return;
1269
1270 if (dmi_get_bios_year() < 2015)
1271 return;
1272
1273 use_acpi_alarm = true;
1274 }
1275 #else
1276 static inline void use_acpi_alarm_quirks(void) { }
1277 #endif
1278
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 03/10] ACPI: LPIT: Use new `pm_suspend_preferred_s2idle`
2022-07-01 2:33 ` [PATCH v3 03/10] ACPI: LPIT: " Mario Limonciello
@ 2022-07-01 13:01 ` kernel test robot
0 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2022-07-01 13:01 UTC (permalink / raw)
To: Mario Limonciello, Rafael J. Wysocki, Len Brown
Cc: kbuild-all, linux-pm, linux-acpi, linux-kernel
Hi Mario,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on drm-misc/drm-misc-next hid/for-next linus/master v5.19-rc4 next-20220701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-a004 (https://download.01.org/0day-ci/archive/20220701/202207012033.3AjPSdQe-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/9603006925839aeb40b8a65adc7be87f4e89813f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
git checkout 9603006925839aeb40b8a65adc7be87f4e89813f
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/acpi/acpi_lpit.c: In function 'lpit_update_residency':
>> drivers/acpi/acpi_lpit.c:113:22: error: implicit declaration of function 'pm_suspend_preferred_s2idle'; did you mean 'pm_suspend_default_s2idle'? [-Werror=implicit-function-declaration]
113 | if (!pm_suspend_preferred_s2idle())
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
| pm_suspend_default_s2idle
drivers/acpi/acpi_lpit.c: At top level:
drivers/acpi/acpi_lpit.c:149:6: warning: no previous prototype for 'acpi_init_lpit' [-Wmissing-prototypes]
149 | void acpi_init_lpit(void)
| ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +113 drivers/acpi/acpi_lpit.c
97
98 static void lpit_update_residency(struct lpit_residency_info *info,
99 struct acpi_lpit_native *lpit_native)
100 {
101 info->frequency = lpit_native->counter_frequency ?
102 lpit_native->counter_frequency : tsc_khz * 1000;
103 if (!info->frequency)
104 info->frequency = 1;
105
106 info->gaddr = lpit_native->residency_counter;
107 if (info->gaddr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
108 info->iomem_addr = ioremap(info->gaddr.address,
109 info->gaddr.bit_width / 8);
110 if (!info->iomem_addr)
111 return;
112
> 113 if (!pm_suspend_preferred_s2idle())
114 return;
115
116 /* Silently fail, if cpuidle attribute group is not present */
117 sysfs_add_file_to_group(&cpu_subsys.dev_root->kobj,
118 &dev_attr_low_power_idle_system_residency_us.attr,
119 "cpuidle");
120 } else if (info->gaddr.space_id == ACPI_ADR_SPACE_FIXED_HARDWARE) {
121 if (!pm_suspend_preferred_s2idle())
122 return;
123
124 /* Silently fail, if cpuidle attribute group is not present */
125 sysfs_add_file_to_group(&cpu_subsys.dev_root->kobj,
126 &dev_attr_low_power_idle_cpu_residency_us.attr,
127 "cpuidle");
128 }
129 }
130
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources
2022-07-01 6:58 ` Greg KH
@ 2022-07-01 18:00 ` Mario Limonciello
0 siblings, 0 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 18:00 UTC (permalink / raw)
To: Greg KH
Cc: Jiri Kosina, Benjamin Tissoires, linux-pm, Richard Gong,
linux-usb, linux-input, linux-kernel
(FYI - combined both of Greg's responses in this message rather than
responding to them separately)
> Note, this is very different than older versions of Windows. Any idea
when this behavior and prescription changed?
Older versions of Windows also didn't support Modern Standby, so I would
hypothesize it's either Windows 8 or Windows 10 that changed it.
> Where are patches 1-9 of this series?
Patch 1 was based on your comment in v2 that there was no helper (it
introduces a helper).
Patches 2-9 are just changes across the kernel to use this helper.
The entire series was submitted to LKML, but individual patches only to
the relevant maintainers for the subsystem. If there is a v4 patch I'll
CC everyone on everything.
Here is a lore link to the whole series:
https://lore.kernel.org/lkml/20220701023328.2783-1-mario.limonciello@amd.com/
On 7/1/22 01:58, Greg KH wrote:
> On Thu, Jun 30, 2022 at 09:33:28PM -0500, Mario Limonciello wrote:
>> The USB HID transport layer doesn't configure mice for wakeup by default.
>> Thus users can not wake system from s2idle via USB mouse. However, users
>> can wake the same system from Modern Standby on Windows with the same USB
>> mouse.
>>
>> Microsoft documentation indicates that all USB mice and touchpads should
>> be waking the system from Modern Standby.
>
> As I said before, their documentation indicates that all _EXTERNAL_ mice
> and touchpads. You forgot about internally connected mice and touchpads
> here, you wouldn't want them to wake up a device asleep, right?
>
Is this actually a thing? I've been in the PC industry a while but
never seen a design that supported Modern Standby/s2idle that opted to
use USB to connect an "internal" touchpad/mouse. Microsoft's own
documentation in the same link advises against it at least too.
"We recommend using HIDI2C for input peripherals whenever possible for
better power efficiency, but this is not a requirement".
But in any case - wakeup from this type of device is a grey area. They
require that internal touchpads connected to I2C be a wake source and
also USB touchpads.
There is nothing on that page prescribing the behavior for an "internal"
USB touchpad.
Reading between the lines given the intent, I would argue if such a
design is created or exists internal USB mice and touchpads should also
be a wake up.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle`
2022-07-01 5:06 ` Damien Le Moal
@ 2022-07-01 18:01 ` Mario Limonciello
0 siblings, 0 replies; 23+ messages in thread
From: Mario Limonciello @ 2022-07-01 18:01 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-pm, linux-ide, linux-kernel
On 7/1/22 00:06, Damien Le Moal wrote:
> On 7/1/22 11:33, Mario Limonciello wrote:
>> Drop the direct check from the FADT and use the helper instead.
>
> Where is this helper defined ? Seeing that this is patch 4/10, I have no
> context for this patch. Please send the whole series next time.
>
My apologies; I should have also included you at least on 1/10, but was
trying to save your inbox for the other 8 patches that were not relevant
to you.
The whole series is on LKML here:
https://lore.kernel.org/lkml/20220701023328.2783-1-mario.limonciello@amd.com/
If there's a v4 I'll include you on 1/10 (unless you also want to be on
the others too).
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> drivers/ata/ahci.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>> index c1eca72b4575..3f79b732dd00 100644
>> --- a/drivers/ata/ahci.c
>> +++ b/drivers/ata/ahci.c
>> @@ -1611,7 +1611,7 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap,
>>
>> #ifdef CONFIG_ACPI
>> if (policy > ATA_LPM_MED_POWER &&
>> - (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) {
>> + pm_suspend_preferred_s2idle()) {
>> if (hpriv->cap & HOST_CAP_PART)
>> policy = ATA_LPM_MIN_POWER_WITH_PARTIAL;
>> else if (hpriv->cap & HOST_CAP_SSC)
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle`
2022-07-01 2:33 ` [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-01 5:06 ` Damien Le Moal
@ 2022-07-02 9:34 ` kernel test robot
1 sibling, 0 replies; 23+ messages in thread
From: kernel test robot @ 2022-07-02 9:34 UTC (permalink / raw)
To: Mario Limonciello, Damien Le Moal
Cc: llvm, kbuild-all, linux-pm, linux-ide, linux-kernel
Hi Mario,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on drm-misc/drm-misc-next hid/for-next linus/master v5.19-rc4 next-20220701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-a011 (https://download.01.org/0day-ci/archive/20220702/202207021730.qxUxPlRq-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project bcd153485ebf07fe79e2b843ed5f1cb74997df1b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/ceb90ecfef1f252424a29b31a547cfc80c8f5135
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/PM-suspend-Introduce-pm_suspend_preferred_s2idle/20220701-103534
git checkout ceb90ecfef1f252424a29b31a547cfc80c8f5135
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/ata/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/ata/ahci.c:1614:6: error: call to undeclared function 'pm_suspend_preferred_s2idle'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
pm_suspend_preferred_s2idle()) {
^
drivers/ata/ahci.c:1614:6: note: did you mean 'pm_suspend_default_s2idle'?
include/linux/suspend.h:343:20: note: 'pm_suspend_default_s2idle' declared here
static inline bool pm_suspend_default_s2idle(void) { return false; }
^
1 error generated.
vim +/pm_suspend_preferred_s2idle +1614 drivers/ata/ahci.c
1595
1596 static void ahci_update_initial_lpm_policy(struct ata_port *ap,
1597 struct ahci_host_priv *hpriv)
1598 {
1599 int policy = CONFIG_SATA_MOBILE_LPM_POLICY;
1600
1601
1602 /* Ignore processing for chipsets that don't use policy */
1603 if (!(hpriv->flags & AHCI_HFLAG_USE_LPM_POLICY))
1604 return;
1605
1606 /* user modified policy via module param */
1607 if (mobile_lpm_policy != -1) {
1608 policy = mobile_lpm_policy;
1609 goto update_policy;
1610 }
1611
1612 #ifdef CONFIG_ACPI
1613 if (policy > ATA_LPM_MED_POWER &&
> 1614 pm_suspend_preferred_s2idle()) {
1615 if (hpriv->cap & HOST_CAP_PART)
1616 policy = ATA_LPM_MIN_POWER_WITH_PARTIAL;
1617 else if (hpriv->cap & HOST_CAP_SSC)
1618 policy = ATA_LPM_MIN_POWER;
1619 }
1620 #endif
1621
1622 update_policy:
1623 if (policy >= ATA_LPM_UNKNOWN && policy <= ATA_LPM_MIN_POWER)
1624 ap->target_lpm_policy = policy;
1625 }
1626
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle`
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
` (8 preceding siblings ...)
2022-07-01 2:33 ` [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources Mario Limonciello
@ 2022-07-12 19:06 ` Rafael J. Wysocki
2022-07-12 20:16 ` Limonciello, Mario
9 siblings, 1 reply; 23+ messages in thread
From: Rafael J. Wysocki @ 2022-07-12 19:06 UTC (permalink / raw)
To: Mario Limonciello
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Linux PM,
Linux Kernel Mailing List
On Fri, Jul 1, 2022 at 4:33 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> Many drivers in the kernel will check the FADT to determine if low
> power idle is supported and use this information to set up a policy
> decision in the driver. To abstract this information from drivers
> introduce a new helper symbol that can indicate this information.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> include/linux/suspend.h | 1 +
> kernel/power/suspend.c | 17 +++++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 70f2921e2e70..9d911e026720 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -305,6 +305,7 @@ static inline bool idle_should_enter_s2idle(void)
> return unlikely(s2idle_state == S2IDLE_STATE_ENTER);
> }
>
> +extern bool pm_suspend_preferred_s2idle(void);
> extern bool pm_suspend_default_s2idle(void);
> extern void __init pm_states_init(void);
> extern void s2idle_set_ops(const struct platform_s2idle_ops *ops);
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 827075944d28..0030e7dfe6cf 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -9,6 +9,7 @@
>
> #define pr_fmt(fmt) "PM: " fmt
>
> +#include <linux/acpi.h>
> #include <linux/string.h>
> #include <linux/delay.h>
> #include <linux/errno.h>
> @@ -61,6 +62,22 @@ static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
> enum s2idle_states __read_mostly s2idle_state;
> static DEFINE_RAW_SPINLOCK(s2idle_lock);
>
> +/**
> + * pm_suspend_preferred_s2idle - Check if suspend-to-idle is the preferred suspend method.
> + *
> + * Return 'true' if suspend-to-idle is preferred by the system designer for the default
> + * suspend method.
> + */
> +bool pm_suspend_preferred_s2idle(void)
> +{
> +#ifdef CONFIG_ACPI
> + return acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0;
> +#else
> + return false;
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(pm_suspend_preferred_s2idle);
First, this is ACPI-specific, so please don't try to generalize it
artificially. This confuses things and doesn't really help.
Second, ACPI_FADT_LOW_POWER_S0 means that "low power S0 idle" is
supported, not that suspend-to-idle is the preferred suspend method in
Linux.
System designers who set that bit in FADT may not even know what
suspend-to-idle is.
But it is good that you have identified the code checking that bit,
because it should not be checked without a valid reason. I need to
review that code and see what's going on in there.
> +
> /**
> * pm_suspend_default_s2idle - Check if suspend-to-idle is the default suspend.
> *
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle`
2022-07-12 19:06 ` [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Rafael J. Wysocki
@ 2022-07-12 20:16 ` Limonciello, Mario
2022-07-13 18:27 ` Rafael J. Wysocki
0 siblings, 1 reply; 23+ messages in thread
From: Limonciello, Mario @ 2022-07-12 20:16 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Len Brown, Linux PM, Linux Kernel Mailing List
On 7/12/2022 14:06, Rafael J. Wysocki wrote:
> On Fri, Jul 1, 2022 at 4:33 AM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> Many drivers in the kernel will check the FADT to determine if low
>> power idle is supported and use this information to set up a policy
>> decision in the driver. To abstract this information from drivers
>> introduce a new helper symbol that can indicate this information.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> include/linux/suspend.h | 1 +
>> kernel/power/suspend.c | 17 +++++++++++++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
>> index 70f2921e2e70..9d911e026720 100644
>> --- a/include/linux/suspend.h
>> +++ b/include/linux/suspend.h
>> @@ -305,6 +305,7 @@ static inline bool idle_should_enter_s2idle(void)
>> return unlikely(s2idle_state == S2IDLE_STATE_ENTER);
>> }
>>
>> +extern bool pm_suspend_preferred_s2idle(void);
>> extern bool pm_suspend_default_s2idle(void);
>> extern void __init pm_states_init(void);
>> extern void s2idle_set_ops(const struct platform_s2idle_ops *ops);
>> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> index 827075944d28..0030e7dfe6cf 100644
>> --- a/kernel/power/suspend.c
>> +++ b/kernel/power/suspend.c
>> @@ -9,6 +9,7 @@
>>
>> #define pr_fmt(fmt) "PM: " fmt
>>
>> +#include <linux/acpi.h>
>> #include <linux/string.h>
>> #include <linux/delay.h>
>> #include <linux/errno.h>
>> @@ -61,6 +62,22 @@ static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
>> enum s2idle_states __read_mostly s2idle_state;
>> static DEFINE_RAW_SPINLOCK(s2idle_lock);
>>
>> +/**
>> + * pm_suspend_preferred_s2idle - Check if suspend-to-idle is the preferred suspend method.
>> + *
>> + * Return 'true' if suspend-to-idle is preferred by the system designer for the default
>> + * suspend method.
>> + */
>> +bool pm_suspend_preferred_s2idle(void)
>> +{
>> +#ifdef CONFIG_ACPI
>> + return acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0;
>> +#else
>> + return false;
>> +#endif
>> +}
>> +EXPORT_SYMBOL_GPL(pm_suspend_preferred_s2idle);
>
> First, this is ACPI-specific, so please don't try to generalize it
> artificially. This confuses things and doesn't really help.
>
> Second, ACPI_FADT_LOW_POWER_S0 means that "low power S0 idle" is
> supported, not that suspend-to-idle is the preferred suspend method in
> Linux.
>
> System designers who set that bit in FADT may not even know what
> suspend-to-idle is.
In "practice" it means that the system has been enabled for Modern
Standby in Windows.
>
> But it is good that you have identified the code checking that bit,
> because it should not be checked without a valid reason. I need to
> review that code and see what's going on in there.
Within this series the intent is to see that the vendor meant the system
to be using Modern Standby on Windows (and implicitly Suspend To Idle on
Linux).
With this I was trying to distinguish intent of the OEM between intent
of the user for helping to declare policy. Maybe the distinction of OEM
and user decision don't really matter though and "mem_sleep_current" is
better to use? I think in a lot of the cases that I outlined I think
that mem_sleep_current can drop right in instead of acpi_gbl_FADT.flags.
If you would like I'm happy to do that and send a new series.
>
>> +
>> /**
>> * pm_suspend_default_s2idle - Check if suspend-to-idle is the default suspend.
>> *
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle`
2022-07-12 20:16 ` Limonciello, Mario
@ 2022-07-13 18:27 ` Rafael J. Wysocki
0 siblings, 0 replies; 23+ messages in thread
From: Rafael J. Wysocki @ 2022-07-13 18:27 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Linux PM,
Linux Kernel Mailing List
On Tue, Jul 12, 2022 at 10:17 PM Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
> On 7/12/2022 14:06, Rafael J. Wysocki wrote:
> > On Fri, Jul 1, 2022 at 4:33 AM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> >>
> >> Many drivers in the kernel will check the FADT to determine if low
> >> power idle is supported and use this information to set up a policy
> >> decision in the driver. To abstract this information from drivers
> >> introduce a new helper symbol that can indicate this information.
> >>
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >> ---
> >> include/linux/suspend.h | 1 +
> >> kernel/power/suspend.c | 17 +++++++++++++++++
> >> 2 files changed, 18 insertions(+)
> >>
> >> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> >> index 70f2921e2e70..9d911e026720 100644
> >> --- a/include/linux/suspend.h
> >> +++ b/include/linux/suspend.h
> >> @@ -305,6 +305,7 @@ static inline bool idle_should_enter_s2idle(void)
> >> return unlikely(s2idle_state == S2IDLE_STATE_ENTER);
> >> }
> >>
> >> +extern bool pm_suspend_preferred_s2idle(void);
> >> extern bool pm_suspend_default_s2idle(void);
> >> extern void __init pm_states_init(void);
> >> extern void s2idle_set_ops(const struct platform_s2idle_ops *ops);
> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> >> index 827075944d28..0030e7dfe6cf 100644
> >> --- a/kernel/power/suspend.c
> >> +++ b/kernel/power/suspend.c
> >> @@ -9,6 +9,7 @@
> >>
> >> #define pr_fmt(fmt) "PM: " fmt
> >>
> >> +#include <linux/acpi.h>
> >> #include <linux/string.h>
> >> #include <linux/delay.h>
> >> #include <linux/errno.h>
> >> @@ -61,6 +62,22 @@ static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
> >> enum s2idle_states __read_mostly s2idle_state;
> >> static DEFINE_RAW_SPINLOCK(s2idle_lock);
> >>
> >> +/**
> >> + * pm_suspend_preferred_s2idle - Check if suspend-to-idle is the preferred suspend method.
> >> + *
> >> + * Return 'true' if suspend-to-idle is preferred by the system designer for the default
> >> + * suspend method.
> >> + */
> >> +bool pm_suspend_preferred_s2idle(void)
> >> +{
> >> +#ifdef CONFIG_ACPI
> >> + return acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0;
> >> +#else
> >> + return false;
> >> +#endif
> >> +}
> >> +EXPORT_SYMBOL_GPL(pm_suspend_preferred_s2idle);
> >
> > First, this is ACPI-specific, so please don't try to generalize it
> > artificially. This confuses things and doesn't really help.
> >
> > Second, ACPI_FADT_LOW_POWER_S0 means that "low power S0 idle" is
> > supported, not that suspend-to-idle is the preferred suspend method in
> > Linux.
> >
> > System designers who set that bit in FADT may not even know what
> > suspend-to-idle is.
>
> In "practice" it means that the system has been enabled for Modern
> Standby in Windows.
>
> >
> > But it is good that you have identified the code checking that bit,
> > because it should not be checked without a valid reason. I need to
> > review that code and see what's going on in there.
>
> Within this series the intent is to see that the vendor meant the system
> to be using Modern Standby on Windows (and implicitly Suspend To Idle on
> Linux).
>
> With this I was trying to distinguish intent of the OEM between intent
> of the user for helping to declare policy. Maybe the distinction of OEM
> and user decision don't really matter though and "mem_sleep_current" is
> better to use? I think in a lot of the cases that I outlined I think
> that mem_sleep_current can drop right in instead of acpi_gbl_FADT.flags.
Checking ACPI_FADT_LOW_POWER_S0 is questionable in the majority of
cases, because if S3 is not supported, that flag is in fact
irrelevant, because Linux will allow suspend-to-idle to be used in
that case regardless of it.
> If you would like I'm happy to do that and send a new series.
I need to look at the individual pieces of code that check
ACPI_FADT_LOW_POWER_S0.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 02/10] ACPI / x86: Use new `pm_suspend_preferred_s2idle`
2022-07-01 2:33 ` [PATCH v3 02/10] ACPI / x86: Use new `pm_suspend_preferred_s2idle` Mario Limonciello
@ 2022-07-13 18:33 ` Rafael J. Wysocki
0 siblings, 0 replies; 23+ messages in thread
From: Rafael J. Wysocki @ 2022-07-13 18:33 UTC (permalink / raw)
To: Mario Limonciello
Cc: Linux Kernel Mailing List, Linux PM, Rafael J. Wysocki,
Len Brown, ACPI Devel Maling List
On Fri, Jul 1, 2022 at 4:33 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> Drop the direct check from the FADT and use the helper instead.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/acpi/x86/s2idle.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 2963229062f8..b1483d5092c1 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -369,7 +369,7 @@ static int lps0_device_attach(struct acpi_device *adev,
> if (lps0_device_handle)
> return 0;
>
> - if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
> + if (!pm_suspend_preferred_s2idle())
This needs to be checked in a different place in this function.
Let me cut a patch for that.
> return 0;
>
> if (acpi_s2idle_vendor_amd()) {
> --
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-07-13 18:33 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-01 2:33 [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 02/10] ACPI / x86: Use new `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-13 18:33 ` Rafael J. Wysocki
2022-07-01 2:33 ` [PATCH v3 03/10] ACPI: LPIT: " Mario Limonciello
2022-07-01 13:01 ` kernel test robot
2022-07-01 2:33 ` [PATCH v3 04/10] ata: ahci: Use `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-01 5:06 ` Damien Le Moal
2022-07-01 18:01 ` Mario Limonciello
2022-07-02 9:34 ` kernel test robot
2022-07-01 2:33 ` [PATCH v3 05/10] rtc: cmos: " Mario Limonciello
2022-07-01 11:50 ` kernel test robot
2022-07-01 2:33 ` [PATCH v3 06/10] thermal: intel: pch: " Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 07/10] drm/amd: " Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 08/10] drm/amd: Use `pm_suspend_default_s2idle` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 09/10] HID: I2C-HID: Use `pm_suspend_preferred_s2idle` Mario Limonciello
2022-07-01 2:33 ` [PATCH v3 10/10] HID: usbhid: Set USB mice as s2idle wakeup resources Mario Limonciello
2022-07-01 6:57 ` Greg KH
2022-07-01 6:58 ` Greg KH
2022-07-01 18:00 ` Mario Limonciello
2022-07-01 11:30 ` kernel test robot
2022-07-12 19:06 ` [PATCH v3 01/10] PM: suspend: Introduce `pm_suspend_preferred_s2idle` Rafael J. Wysocki
2022-07-12 20:16 ` Limonciello, Mario
2022-07-13 18:27 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).