* [PATCH 0/2] Suspend-to-idle fixes for Surface Pro3
@ 2018-01-10 12:22 Rafael J. Wysocki
2018-01-10 12:26 ` [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems Rafael J. Wysocki
2018-01-10 12:26 ` [PATCH 2/2] platform/x86: surfacepro3: Support for wakeup from suspend-to-idle Rafael J. Wysocki
0 siblings, 2 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2018-01-10 12:22 UTC (permalink / raw)
To: Linux ACPI
Cc: Andy Shevchenko, Darren Hart, LKML, Linux PM, Platform Driver,
Valentin Manea
Hi All,
After some changes made during the 4.13 cycle, power button doesn't wake up
the system from suspend-to-idle any more on Surface Pro3.
It turns out that the platform expects EC events to be processed while
suspended for this to work (patch [1/2]) and the button driver needs to
report wakeup events properly too (patch [2/2]).
Thanks,
Rafael
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 12:22 [PATCH 0/2] Suspend-to-idle fixes for Surface Pro3 Rafael J. Wysocki
@ 2018-01-10 12:26 ` Rafael J. Wysocki
2018-01-10 13:24 ` Andy Shevchenko
2018-01-10 17:38 ` Mario.Limonciello
2018-01-10 12:26 ` [PATCH 2/2] platform/x86: surfacepro3: Support for wakeup from suspend-to-idle Rafael J. Wysocki
1 sibling, 2 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2018-01-10 12:26 UTC (permalink / raw)
To: Linux ACPI
Cc: Andy Shevchenko, Darren Hart, LKML, Linux PM, Platform Driver,
Valentin Manea
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
functions in their Low Power S0 Idle _DSM, but still expect EC
events to be processed in the suspend-to-idle state for power button
wakeup (among other things) to work. Surface Pro3 turns out to be
one of them.
Fortunately, it still provides Low Power S0 Idle _DSM with the screen
on/off functions supported, so modify the ACPI suspend-to-idle to use
the Low Power S0 Idle code path for all systems supporting the
ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
Potentially, that will cause more systems to use suspend-to-idle by
default, so some future corrections may be necessary if it leads
to issues, but let it remain more straightforward for now.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
Reported-by: Valentin Manea <valy@mrs.ro>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/sleep.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
#define ACPI_LPS0_ENTRY 5
#define ACPI_LPS0_EXIT 6
-#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 << ACPI_LPS0_EXIT))
+#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) | (1 << ACPI_LPS0_SCREEN_ON))
+#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 << ACPI_LPS0_EXIT))
static acpi_handle lps0_device_handle;
static guid_t lps0_dsm_guid;
@@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
char bitmask = *(char *)out_obj->buffer.pointer;
- if ((bitmask & ACPI_S2IDLE_FUNC_MASK) == ACPI_S2IDLE_FUNC_MASK) {
+ if ((bitmask & ACPI_LPS0_S2I_MASK) == ACPI_LPS0_S2I_MASK ||
+ (bitmask & ACPI_LPS0_SCREEN_MASK) == ACPI_LPS0_SCREEN_MASK) {
lps0_dsm_func_mask = bitmask;
lps0_device_handle = adev->handle;
/*
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] platform/x86: surfacepro3: Support for wakeup from suspend-to-idle
2018-01-10 12:22 [PATCH 0/2] Suspend-to-idle fixes for Surface Pro3 Rafael J. Wysocki
2018-01-10 12:26 ` [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems Rafael J. Wysocki
@ 2018-01-10 12:26 ` Rafael J. Wysocki
2018-01-10 13:26 ` Andy Shevchenko
1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2018-01-10 12:26 UTC (permalink / raw)
To: Linux ACPI
Cc: Andy Shevchenko, Darren Hart, LKML, Linux PM, Platform Driver,
Valentin Manea
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Modify surface_button_notify() to make it wake up the system from
suspend-to-idle (by reporting "hard" wakeup events while suspended)
and add wakeup initialization to surface_button_add() for wakeup
events reported by this driver to work at all.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
Reported-by: Valentin Manea <valy@mrs.ro>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/platform/x86/surfacepro3_button.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/platform/x86/surfacepro3_button.c
===================================================================
--- linux-pm.orig/drivers/platform/x86/surfacepro3_button.c
+++ linux-pm/drivers/platform/x86/surfacepro3_button.c
@@ -119,7 +119,7 @@ static void surface_button_notify(struct
if (key_code == KEY_RESERVED)
return;
if (pressed)
- pm_wakeup_event(&device->dev, 0);
+ pm_wakeup_dev_event(&device->dev, 0, button->suspended);
if (button->suspended)
return;
input_report_key(input, key_code, pressed?1:0);
@@ -185,6 +185,8 @@ static int surface_button_add(struct acp
error = input_register_device(input);
if (error)
goto err_free_input;
+
+ device_init_wakeup(&device->dev, true);
dev_info(&device->dev,
"%s [%s]\n", name, acpi_device_bid(device));
return 0;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 12:26 ` [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems Rafael J. Wysocki
@ 2018-01-10 13:24 ` Andy Shevchenko
2018-01-10 22:25 ` Rafael J. Wysocki
2018-01-11 1:13 ` [PATCH v2 " Rafael J. Wysocki
2018-01-10 17:38 ` Mario.Limonciello
1 sibling, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2018-01-10 13:24 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: Darren Hart, LKML, Linux PM, Platform Driver, Valentin Manea
On Wed, 2018-01-10 at 13:26 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
> functions in their Low Power S0 Idle _DSM, but still expect EC
> events to be processed in the suspend-to-idle state for power button
> wakeup (among other things) to work. Surface Pro3 turns out to be
> one of them.
>
> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
> on/off functions supported, so modify the ACPI suspend-to-idle to use
> the Low Power S0 Idle code path for all systems supporting the
> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
>
> Potentially, that will cause more systems to use suspend-to-idle by
> default, so some future corrections may be necessary if it leads
> to issues, but let it remain more straightforward for now.
> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> ACPI_LPS0_EXIT))
> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) |
> (1 << ACPI_LPS0_SCREEN_ON))
> +#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> ACPI_LPS0_EXIT))
Just a nitpick: Can we leave S2IDLE instead of S2I?
Would it make sense for potential code readers?
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] platform/x86: surfacepro3: Support for wakeup from suspend-to-idle
2018-01-10 12:26 ` [PATCH 2/2] platform/x86: surfacepro3: Support for wakeup from suspend-to-idle Rafael J. Wysocki
@ 2018-01-10 13:26 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2018-01-10 13:26 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: Darren Hart, LKML, Linux PM, Platform Driver, Valentin Manea
On Wed, 2018-01-10 at 13:26 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Modify surface_button_notify() to make it wake up the system from
> suspend-to-idle (by reporting "hard" wakeup events while suspended)
> and add wakeup initialization to surface_button_add() for wakeup
> events reported by this driver to work at all.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
> Reported-by: Valentin Manea <valy@mrs.ro>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Change seems sane to me.
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
(in case you would like to push it through PM tree, otherwise I can take
it)
> ---
> drivers/platform/x86/surfacepro3_button.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/platform/x86/surfacepro3_button.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/x86/surfacepro3_button.c
> +++ linux-pm/drivers/platform/x86/surfacepro3_button.c
> @@ -119,7 +119,7 @@ static void surface_button_notify(struct
> if (key_code == KEY_RESERVED)
> return;
> if (pressed)
> - pm_wakeup_event(&device->dev, 0);
> + pm_wakeup_dev_event(&device->dev, 0, button-
> >suspended);
> if (button->suspended)
> return;
> input_report_key(input, key_code, pressed?1:0);
> @@ -185,6 +185,8 @@ static int surface_button_add(struct acp
> error = input_register_device(input);
> if (error)
> goto err_free_input;
> +
> + device_init_wakeup(&device->dev, true);
> dev_info(&device->dev,
> "%s [%s]\n", name, acpi_device_bid(device));
> return 0;
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 12:26 ` [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems Rafael J. Wysocki
@ 2018-01-10 17:38 ` Mario.Limonciello
2018-01-10 17:38 ` Mario.Limonciello
1 sibling, 0 replies; 14+ messages in thread
From: Mario.Limonciello @ 2018-01-10 17:38 UTC (permalink / raw)
To: rjw, linux-acpi
Cc: andriy.shevchenko, dvhart, linux-kernel, linux-pm,
platform-driver-x86, valy
> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> Sent: Wednesday, January 10, 2018 6:26 AM
> To: Linux ACPI <linux-acpi@vger.kernel.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux PM <linux-
> pm@vger.kernel.org>; Platform Driver <platform-driver-x86@vger.kernel.org>;
> Valentin Manea <valy@mrs.ro>
> Subject: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
> functions in their Low Power S0 Idle _DSM, but still expect EC
> events to be processed in the suspend-to-idle state for power button
> wakeup (among other things) to work. Surface Pro3 turns out to be
> one of them.
>
> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
> on/off functions supported, so modify the ACPI suspend-to-idle to use
> the Low Power S0 Idle code path for all systems supporting the
> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
>
> Potentially, that will cause more systems to use suspend-to-idle by
> default, so some future corrections may be necessary if it leads
> to issues, but let it remain more straightforward for now.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
> Reported-by: Valentin Manea <valy@mrs.ro>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/sleep.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
> #define ACPI_LPS0_ENTRY 5
> #define ACPI_LPS0_EXIT 6
>
> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> ACPI_LPS0_EXIT))
> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) | (1 <<
> ACPI_LPS0_SCREEN_ON))
> +#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 << ACPI_LPS0_EXIT))
>
> static acpi_handle lps0_device_handle;
> static guid_t lps0_dsm_guid;
> @@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
> if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
> char bitmask = *(char *)out_obj->buffer.pointer;
>
> - if ((bitmask & ACPI_S2IDLE_FUNC_MASK) ==
> ACPI_S2IDLE_FUNC_MASK) {
> + if ((bitmask & ACPI_LPS0_S2I_MASK) == ACPI_LPS0_S2I_MASK ||
> + (bitmask & ACPI_LPS0_SCREEN_MASK) ==
> ACPI_LPS0_SCREEN_MASK) {
> lps0_dsm_func_mask = bitmask;
> lps0_device_handle = adev->handle;
> /*
In making this change I believe you'll need to cache the values that you found from the
function mask to test them later too.
Here:
https://github.com/torvalds/linux/blob/master/drivers/acpi/sleep.c#L943
This is because later on both ACPI_LPS0_SCREEN_OFF and ACPI_LPS0_ENTRY are called
whether or not they both exist. Fortunately looking at the DSDT in the attached bug nothing
happens if calling the undefined Arg2 == 0x05/Arg2 == 0x06 but that might not always
be the case.
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
@ 2018-01-10 17:38 ` Mario.Limonciello
0 siblings, 0 replies; 14+ messages in thread
From: Mario.Limonciello @ 2018-01-10 17:38 UTC (permalink / raw)
To: rjw, linux-acpi
Cc: andriy.shevchenko, dvhart, linux-kernel, linux-pm,
platform-driver-x86, valy
> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> Sent: Wednesday, January 10, 2018 6:26 AM
> To: Linux ACPI <linux-acpi@vger.kernel.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux PM <linux-
> pm@vger.kernel.org>; Platform Driver <platform-driver-x86@vger.kernel.org>;
> Valentin Manea <valy@mrs.ro>
> Subject: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
> functions in their Low Power S0 Idle _DSM, but still expect EC
> events to be processed in the suspend-to-idle state for power button
> wakeup (among other things) to work. Surface Pro3 turns out to be
> one of them.
>
> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
> on/off functions supported, so modify the ACPI suspend-to-idle to use
> the Low Power S0 Idle code path for all systems supporting the
> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
>
> Potentially, that will cause more systems to use suspend-to-idle by
> default, so some future corrections may be necessary if it leads
> to issues, but let it remain more straightforward for now.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
> Reported-by: Valentin Manea <valy@mrs.ro>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/sleep.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
> #define ACPI_LPS0_ENTRY 5
> #define ACPI_LPS0_EXIT 6
>
> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> ACPI_LPS0_EXIT))
> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) | (1 <<
> ACPI_LPS0_SCREEN_ON))
> +#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 << ACPI_LPS0_EXIT))
>
> static acpi_handle lps0_device_handle;
> static guid_t lps0_dsm_guid;
> @@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
> if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
> char bitmask = *(char *)out_obj->buffer.pointer;
>
> - if ((bitmask & ACPI_S2IDLE_FUNC_MASK) ==
> ACPI_S2IDLE_FUNC_MASK) {
> + if ((bitmask & ACPI_LPS0_S2I_MASK) == ACPI_LPS0_S2I_MASK ||
> + (bitmask & ACPI_LPS0_SCREEN_MASK) ==
> ACPI_LPS0_SCREEN_MASK) {
> lps0_dsm_func_mask = bitmask;
> lps0_device_handle = adev->handle;
> /*
In making this change I believe you'll need to cache the values that you found from the
function mask to test them later too.
Here:
https://github.com/torvalds/linux/blob/master/drivers/acpi/sleep.c#L943
This is because later on both ACPI_LPS0_SCREEN_OFF and ACPI_LPS0_ENTRY are called
whether or not they both exist. Fortunately looking at the DSDT in the attached bug nothing
happens if calling the undefined Arg2 == 0x05/Arg2 == 0x06 but that might not always
be the case.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 17:38 ` Mario.Limonciello
(?)
@ 2018-01-10 22:22 ` Rafael J. Wysocki
2018-01-11 15:51 ` Mario.Limonciello
-1 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2018-01-10 22:22 UTC (permalink / raw)
To: Mario Limonciello
Cc: Rafael J. Wysocki, ACPI Devel Maling List, Andy Shevchenko,
Darren Hart, Linux Kernel Mailing List, Linux PM,
Platform Driver, valy
On Wed, Jan 10, 2018 at 6:38 PM, <Mario.Limonciello@dell.com> wrote:
>
>
>> -----Original Message-----
>> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
>> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
>> Sent: Wednesday, January 10, 2018 6:26 AM
>> To: Linux ACPI <linux-acpi@vger.kernel.org>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
>> <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux PM <linux-
>> pm@vger.kernel.org>; Platform Driver <platform-driver-x86@vger.kernel.org>;
>> Valentin Manea <valy@mrs.ro>
>> Subject: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
>>
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
>> functions in their Low Power S0 Idle _DSM, but still expect EC
>> events to be processed in the suspend-to-idle state for power button
>> wakeup (among other things) to work. Surface Pro3 turns out to be
>> one of them.
>>
>> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
>> on/off functions supported, so modify the ACPI suspend-to-idle to use
>> the Low Power S0 Idle code path for all systems supporting the
>> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
>> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
>>
>> Potentially, that will cause more systems to use suspend-to-idle by
>> default, so some future corrections may be necessary if it leads
>> to issues, but let it remain more straightforward for now.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
>> Reported-by: Valentin Manea <valy@mrs.ro>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> ---
>> drivers/acpi/sleep.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> Index: linux-pm/drivers/acpi/sleep.c
>> ===================================================================
>> --- linux-pm.orig/drivers/acpi/sleep.c
>> +++ linux-pm/drivers/acpi/sleep.c
>> @@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
>> #define ACPI_LPS0_ENTRY 5
>> #define ACPI_LPS0_EXIT 6
>>
>> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
>> ACPI_LPS0_EXIT))
>> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) | (1 <<
>> ACPI_LPS0_SCREEN_ON))
>> +#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 << ACPI_LPS0_EXIT))
>>
>> static acpi_handle lps0_device_handle;
>> static guid_t lps0_dsm_guid;
>> @@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
>> if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
>> char bitmask = *(char *)out_obj->buffer.pointer;
>>
>> - if ((bitmask & ACPI_S2IDLE_FUNC_MASK) ==
>> ACPI_S2IDLE_FUNC_MASK) {
>> + if ((bitmask & ACPI_LPS0_S2I_MASK) == ACPI_LPS0_S2I_MASK ||
>> + (bitmask & ACPI_LPS0_SCREEN_MASK) ==
>> ACPI_LPS0_SCREEN_MASK) {
>> lps0_dsm_func_mask = bitmask;
>> lps0_device_handle = adev->handle;
>> /*
>
> In making this change I believe you'll need to cache the values that you found from the
> function mask to test them later too.
But that's what lps0_dsm_func_mask is for if I understand you correctly.
> Here:
> https://github.com/torvalds/linux/blob/master/drivers/acpi/sleep.c#L943
>
> This is because later on both ACPI_LPS0_SCREEN_OFF and ACPI_LPS0_ENTRY are called
> whether or not they both exist.
No, that's not the case.
acpi_sleep_run_lps0_dsm() checks if the given function is there in the
mask returned by function 0 and it doesn't evaluate the _DSM
otherwise.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 13:24 ` Andy Shevchenko
@ 2018-01-10 22:25 ` Rafael J. Wysocki
2018-01-11 12:54 ` Andy Shevchenko
2018-01-11 1:13 ` [PATCH v2 " Rafael J. Wysocki
1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2018-01-10 22:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rafael J. Wysocki, Linux ACPI, Darren Hart, LKML, Linux PM,
Platform Driver, Valentin Manea
On Wed, Jan 10, 2018 at 2:24 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Wed, 2018-01-10 at 13:26 +0100, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
>> functions in their Low Power S0 Idle _DSM, but still expect EC
>> events to be processed in the suspend-to-idle state for power button
>> wakeup (among other things) to work. Surface Pro3 turns out to be
>> one of them.
>>
>> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
>> on/off functions supported, so modify the ACPI suspend-to-idle to use
>> the Low Power S0 Idle code path for all systems supporting the
>> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
>> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
>>
>> Potentially, that will cause more systems to use suspend-to-idle by
>> default, so some future corrections may be necessary if it leads
>> to issues, but let it remain more straightforward for now.
>
>> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
>> ACPI_LPS0_EXIT))
>> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) |
>> (1 << ACPI_LPS0_SCREEN_ON))
>
>> +#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
>> ACPI_LPS0_EXIT))
>
> Just a nitpick: Can we leave S2IDLE instead of S2I?
> Would it make sense for potential code readers?
I wanted it to be shorter, but if that is a problem, I'd rather call
it PLATFORM than S2IDLE (as technically they are related to the
low-power mode of the platform).
I'll send an update shortly.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 13:24 ` Andy Shevchenko
2018-01-10 22:25 ` Rafael J. Wysocki
@ 2018-01-11 1:13 ` Rafael J. Wysocki
2018-01-11 12:55 ` Andy Shevchenko
1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2018-01-11 1:13 UTC (permalink / raw)
To: Linux ACPI
Cc: Andy Shevchenko, Darren Hart, LKML, Linux PM, Platform Driver,
Valentin Manea
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
functions in their Low Power S0 Idle _DSM, but still expect EC
events to be processed in the suspend-to-idle state for power button
wakeup (among other things) to work. Surface Pro3 turns out to be
one of them.
Fortunately, it still provides Low Power S0 Idle _DSM with the screen
on/off functions supported, so modify the ACPI suspend-to-idle to use
the Low Power S0 Idle code path for all systems supporting the
ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
Potentially, that will cause more systems to use suspend-to-idle by
default, so some future corrections may be necessary if it leads
to issues, but let it remain more straightforward for now.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389#add_comment
Reported-by: Valentin Manea <valy@mrs.ro>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
-> v2: Rename ACPI_LPS0_S2I_MASK to ACPI_LPS0_PLATFORM_MASK.
---
drivers/acpi/sleep.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
#define ACPI_LPS0_ENTRY 5
#define ACPI_LPS0_EXIT 6
-#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 << ACPI_LPS0_EXIT))
+#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) | (1 << ACPI_LPS0_SCREEN_ON))
+#define ACPI_LPS0_PLATFORM_MASK ((1 << ACPI_LPS0_ENTRY) | (1 << ACPI_LPS0_EXIT))
static acpi_handle lps0_device_handle;
static guid_t lps0_dsm_guid;
@@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
char bitmask = *(char *)out_obj->buffer.pointer;
- if ((bitmask & ACPI_S2IDLE_FUNC_MASK) == ACPI_S2IDLE_FUNC_MASK) {
+ if ((bitmask & ACPI_LPS0_PLATFORM_MASK) == ACPI_LPS0_PLATFORM_MASK ||
+ (bitmask & ACPI_LPS0_SCREEN_MASK) == ACPI_LPS0_SCREEN_MASK) {
lps0_dsm_func_mask = bitmask;
lps0_device_handle = adev->handle;
/*
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 22:25 ` Rafael J. Wysocki
@ 2018-01-11 12:54 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2018-01-11 12:54 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Linux ACPI, Darren Hart, LKML, Linux PM,
Platform Driver, Valentin Manea
On Wed, 2018-01-10 at 23:25 +0100, Rafael J. Wysocki wrote:
> On Wed, Jan 10, 2018 at 2:24 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, 2018-01-10 at 13:26 +0100, Rafael J. Wysocki wrote:
> > Just a nitpick: Can we leave S2IDLE instead of S2I?
> > Would it make sense for potential code readers?
>
> I wanted it to be shorter, but if that is a problem, I'd rather call
> it PLATFORM than S2IDLE (as technically they are related to the
> low-power mode of the platform).
It's not problem per se, though without a context it would take time to
get into S2I acronym from the code for not familiar reader.
> I'll send an update shortly.
Thanks!
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-11 1:13 ` [PATCH v2 " Rafael J. Wysocki
@ 2018-01-11 12:55 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2018-01-11 12:55 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux ACPI
Cc: Darren Hart, LKML, Linux PM, Platform Driver, Valentin Manea
On Thu, 2018-01-11 at 02:13 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
> functions in their Low Power S0 Idle _DSM, but still expect EC
> events to be processed in the suspend-to-idle state for power button
> wakeup (among other things) to work. Surface Pro3 turns out to be
> one of them.
>
> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
> on/off functions supported, so modify the ACPI suspend-to-idle to use
> the Low Power S0 Idle code path for all systems supporting the
> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
>
> Potentially, that will cause more systems to use suspend-to-idle by
> default, so some future corrections may be necessary if it leads
> to issues, but let it remain more straightforward for now.
>
FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389#add_comment
> Reported-by: Valentin Manea <valy@mrs.ro>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> -> v2: Rename ACPI_LPS0_S2I_MASK to ACPI_LPS0_PLATFORM_MASK.
>
> ---
> drivers/acpi/sleep.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
> #define ACPI_LPS0_ENTRY 5
> #define ACPI_LPS0_EXIT 6
>
> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> ACPI_LPS0_EXIT))
> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) |
> (1 << ACPI_LPS0_SCREEN_ON))
> +#define ACPI_LPS0_PLATFORM_MASK ((1 << ACPI_LPS0_ENTRY) | (1
> << ACPI_LPS0_EXIT))
>
> static acpi_handle lps0_device_handle;
> static guid_t lps0_dsm_guid;
> @@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
> if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
> char bitmask = *(char *)out_obj->buffer.pointer;
>
> - if ((bitmask & ACPI_S2IDLE_FUNC_MASK) ==
> ACPI_S2IDLE_FUNC_MASK) {
> + if ((bitmask & ACPI_LPS0_PLATFORM_MASK) ==
> ACPI_LPS0_PLATFORM_MASK ||
> + (bitmask & ACPI_LPS0_SCREEN_MASK) ==
> ACPI_LPS0_SCREEN_MASK) {
> lps0_dsm_func_mask = bitmask;
> lps0_device_handle = adev->handle;
> /*
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
2018-01-10 22:22 ` Rafael J. Wysocki
@ 2018-01-11 15:51 ` Mario.Limonciello
0 siblings, 0 replies; 14+ messages in thread
From: Mario.Limonciello @ 2018-01-11 15:51 UTC (permalink / raw)
To: rafael
Cc: rjw, linux-acpi, andriy.shevchenko, dvhart, linux-kernel,
linux-pm, platform-driver-x86, valy
> -----Original Message-----
> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J.
> Wysocki
> Sent: Wednesday, January 10, 2018 4:23 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; ACPI Devel Maling List <linux-
> acpi@vger.kernel.org>; Andy Shevchenko <andriy.shevchenko@linux.intel.com>;
> Darren Hart <dvhart@infradead.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; Platform Driver
> <platform-driver-x86@vger.kernel.org>; valy@mrs.ro
> Subject: Re: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
>
> On Wed, Jan 10, 2018 at 6:38 PM, <Mario.Limonciello@dell.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> >> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> >> Sent: Wednesday, January 10, 2018 6:26 AM
> >> To: Linux ACPI <linux-acpi@vger.kernel.org>
> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> >> <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux PM
> <linux-
> >> pm@vger.kernel.org>; Platform Driver <platform-driver-x86@vger.kernel.org>;
> >> Valentin Manea <valy@mrs.ro>
> >> Subject: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
> >>
> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>
> >> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
> >> functions in their Low Power S0 Idle _DSM, but still expect EC
> >> events to be processed in the suspend-to-idle state for power button
> >> wakeup (among other things) to work. Surface Pro3 turns out to be
> >> one of them.
> >>
> >> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
> >> on/off functions supported, so modify the ACPI suspend-to-idle to use
> >> the Low Power S0 Idle code path for all systems supporting the
> >> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
> >> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
> >>
> >> Potentially, that will cause more systems to use suspend-to-idle by
> >> default, so some future corrections may be necessary if it leads
> >> to issues, but let it remain more straightforward for now.
> >>
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
> >> Reported-by: Valentin Manea <valy@mrs.ro>
> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> ---
> >> drivers/acpi/sleep.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> Index: linux-pm/drivers/acpi/sleep.c
> >> ===================================================================
> >> --- linux-pm.orig/drivers/acpi/sleep.c
> >> +++ linux-pm/drivers/acpi/sleep.c
> >> @@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
> >> #define ACPI_LPS0_ENTRY 5
> >> #define ACPI_LPS0_EXIT 6
> >>
> >> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> >> ACPI_LPS0_EXIT))
> >> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) | (1 <<
> >> ACPI_LPS0_SCREEN_ON))
> >> +#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> ACPI_LPS0_EXIT))
> >>
> >> static acpi_handle lps0_device_handle;
> >> static guid_t lps0_dsm_guid;
> >> @@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
> >> if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
> >> char bitmask = *(char *)out_obj->buffer.pointer;
> >>
> >> - if ((bitmask & ACPI_S2IDLE_FUNC_MASK) ==
> >> ACPI_S2IDLE_FUNC_MASK) {
> >> + if ((bitmask & ACPI_LPS0_S2I_MASK) == ACPI_LPS0_S2I_MASK ||
> >> + (bitmask & ACPI_LPS0_SCREEN_MASK) ==
> >> ACPI_LPS0_SCREEN_MASK) {
> >> lps0_dsm_func_mask = bitmask;
> >> lps0_device_handle = adev->handle;
> >> /*
> >
> > In making this change I believe you'll need to cache the values that you found
> from the
> > function mask to test them later too.
>
> But that's what lps0_dsm_func_mask is for if I understand you correctly.
>
> > Here:
> > https://github.com/torvalds/linux/blob/master/drivers/acpi/sleep.c#L943
> >
> > This is because later on both ACPI_LPS0_SCREEN_OFF and ACPI_LPS0_ENTRY are
> called
> > whether or not they both exist.
>
> No, that's not the case.
>
> acpi_sleep_run_lps0_dsm() checks if the given function is there in the
> mask returned by function 0 and it doesn't evaluate the _DSM
> otherwise.
>
> Thanks,
> Rafael
Thanks yes, I see this more closely now you're right.
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
@ 2018-01-11 15:51 ` Mario.Limonciello
0 siblings, 0 replies; 14+ messages in thread
From: Mario.Limonciello @ 2018-01-11 15:51 UTC (permalink / raw)
To: rafael
Cc: rjw, linux-acpi, andriy.shevchenko, dvhart, linux-kernel,
linux-pm, platform-driver-x86, valy
> -----Original Message-----
> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J.
> Wysocki
> Sent: Wednesday, January 10, 2018 4:23 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; ACPI Devel Maling List <linux-
> acpi@vger.kernel.org>; Andy Shevchenko <andriy.shevchenko@linux.intel.com>;
> Darren Hart <dvhart@infradead.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; Platform Driver
> <platform-driver-x86@vger.kernel.org>; valy@mrs.ro
> Subject: Re: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
>
> On Wed, Jan 10, 2018 at 6:38 PM, <Mario.Limonciello@dell.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> >> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> >> Sent: Wednesday, January 10, 2018 6:26 AM
> >> To: Linux ACPI <linux-acpi@vger.kernel.org>
> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Darren Hart
> >> <dvhart@infradead.org>; LKML <linux-kernel@vger.kernel.org>; Linux PM
> <linux-
> >> pm@vger.kernel.org>; Platform Driver <platform-driver-x86@vger.kernel.org>;
> >> Valentin Manea <valy@mrs.ro>
> >> Subject: [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems
> >>
> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>
> >> Some systems don't support the ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT
> >> functions in their Low Power S0 Idle _DSM, but still expect EC
> >> events to be processed in the suspend-to-idle state for power button
> >> wakeup (among other things) to work. Surface Pro3 turns out to be
> >> one of them.
> >>
> >> Fortunately, it still provides Low Power S0 Idle _DSM with the screen
> >> on/off functions supported, so modify the ACPI suspend-to-idle to use
> >> the Low Power S0 Idle code path for all systems supporting the
> >> ACPI_LPS0_ENTRY and ACPI_LPS0_EXIT or the ACPI_LPS0_SCREEN_OFF and
> >> ACPI_LPS0_SCREEN_ON functions in their Low Power S0 Idle _DSM.
> >>
> >> Potentially, that will cause more systems to use suspend-to-idle by
> >> default, so some future corrections may be necessary if it leads
> >> to issues, but let it remain more straightforward for now.
> >>
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198389
> >> Reported-by: Valentin Manea <valy@mrs.ro>
> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> ---
> >> drivers/acpi/sleep.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> Index: linux-pm/drivers/acpi/sleep.c
> >> ===================================================================
> >> --- linux-pm.orig/drivers/acpi/sleep.c
> >> +++ linux-pm/drivers/acpi/sleep.c
> >> @@ -707,7 +707,8 @@ static const struct acpi_device_id lps0_
> >> #define ACPI_LPS0_ENTRY 5
> >> #define ACPI_LPS0_EXIT 6
> >>
> >> -#define ACPI_S2IDLE_FUNC_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> >> ACPI_LPS0_EXIT))
> >> +#define ACPI_LPS0_SCREEN_MASK ((1 << ACPI_LPS0_SCREEN_OFF) | (1 <<
> >> ACPI_LPS0_SCREEN_ON))
> >> +#define ACPI_LPS0_S2I_MASK ((1 << ACPI_LPS0_ENTRY) | (1 <<
> ACPI_LPS0_EXIT))
> >>
> >> static acpi_handle lps0_device_handle;
> >> static guid_t lps0_dsm_guid;
> >> @@ -910,7 +911,8 @@ static int lps0_device_attach(struct acp
> >> if (out_obj && out_obj->type == ACPI_TYPE_BUFFER) {
> >> char bitmask = *(char *)out_obj->buffer.pointer;
> >>
> >> - if ((bitmask & ACPI_S2IDLE_FUNC_MASK) ==
> >> ACPI_S2IDLE_FUNC_MASK) {
> >> + if ((bitmask & ACPI_LPS0_S2I_MASK) == ACPI_LPS0_S2I_MASK ||
> >> + (bitmask & ACPI_LPS0_SCREEN_MASK) ==
> >> ACPI_LPS0_SCREEN_MASK) {
> >> lps0_dsm_func_mask = bitmask;
> >> lps0_device_handle = adev->handle;
> >> /*
> >
> > In making this change I believe you'll need to cache the values that you found
> from the
> > function mask to test them later too.
>
> But that's what lps0_dsm_func_mask is for if I understand you correctly.
>
> > Here:
> > https://github.com/torvalds/linux/blob/master/drivers/acpi/sleep.c#L943
> >
> > This is because later on both ACPI_LPS0_SCREEN_OFF and ACPI_LPS0_ENTRY are
> called
> > whether or not they both exist.
>
> No, that's not the case.
>
> acpi_sleep_run_lps0_dsm() checks if the given function is there in the
> mask returned by function 0 and it doesn't evaluate the _DSM
> otherwise.
>
> Thanks,
> Rafael
Thanks yes, I see this more closely now you're right.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-01-11 15:51 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10 12:22 [PATCH 0/2] Suspend-to-idle fixes for Surface Pro3 Rafael J. Wysocki
2018-01-10 12:26 ` [PATCH 1/2] ACPI / PM: Use Low Power S0 Idle on more systems Rafael J. Wysocki
2018-01-10 13:24 ` Andy Shevchenko
2018-01-10 22:25 ` Rafael J. Wysocki
2018-01-11 12:54 ` Andy Shevchenko
2018-01-11 1:13 ` [PATCH v2 " Rafael J. Wysocki
2018-01-11 12:55 ` Andy Shevchenko
2018-01-10 17:38 ` [PATCH " Mario.Limonciello
2018-01-10 17:38 ` Mario.Limonciello
2018-01-10 22:22 ` Rafael J. Wysocki
2018-01-11 15:51 ` Mario.Limonciello
2018-01-11 15:51 ` Mario.Limonciello
2018-01-10 12:26 ` [PATCH 2/2] platform/x86: surfacepro3: Support for wakeup from suspend-to-idle Rafael J. Wysocki
2018-01-10 13:26 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.