All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
@ 2023-04-20 16:09 Mario Limonciello
  2023-04-21  6:15 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Mario Limonciello @ 2023-04-20 16:09 UTC (permalink / raw)
  To: rafael, linux-kernel
  Cc: linux-pm, Shyam-sundar.S-k, Mario Limonciello, Len Brown, linux-acpi

When utilizing the Microsoft _DSM there are six calls which correspond
to the following states:

1) Idle (Screen on)
2) Idle (Screen off)
3) Low Power
4) Modern Standby

Currently Linux compresses these and treats this as two states:

1) Awake
2) s2idle

That is when the system has woken from s2idle from a pure ACPI event
the _DSM state is still Modern Standby at this time, and so some
firmware will not run other code that communicates with the EC.

To fix this, track the active state in the s2idle code and ensure that
"Modern Standby enter" is called after the ACPI SCI has been processed
from `acpi_s2idle_wake` and every time that the system is woken up call
"Modern Standby exit".

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/x86/s2idle.c | 85 +++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index e499c60c4579..1772d981c5e9 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -59,6 +59,7 @@ static int lps0_dsm_func_mask;
 
 static guid_t lps0_dsm_guid_microsoft;
 static int lps0_dsm_func_mask_microsoft;
+static int lps0_dsm_state;
 
 /* Device constraint entry structure */
 struct lpi_device_info {
@@ -320,6 +321,44 @@ static void lpi_check_constraints(void)
 	}
 }
 
+static bool acpi_s2idle_vendor_amd(void)
+{
+	return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
+}
+
+static const char *acpi_sleep_dsm_state_to_str(unsigned int state)
+{
+	if (lps0_dsm_func_mask_microsoft || !acpi_s2idle_vendor_amd()) {
+		switch (state) {
+		case ACPI_LPS0_SCREEN_OFF:
+			return "screen off";
+		case ACPI_LPS0_SCREEN_ON:
+			return "screen on";
+		case ACPI_LPS0_ENTRY:
+			return "lps0 entry";
+		case ACPI_LPS0_EXIT:
+			return "lps0 exit";
+		case ACPI_LPS0_MS_ENTRY:
+			return "lps0 ms entry";
+		case ACPI_LPS0_MS_EXIT:
+			return "lps0 ms exit";
+		};
+	} else {
+		switch (state) {
+		case ACPI_LPS0_SCREEN_ON_AMD:
+			return "screen on";
+		case ACPI_LPS0_SCREEN_OFF_AMD:
+			return "screen off";
+		case ACPI_LPS0_ENTRY_AMD:
+			return "lps0 entry";
+		case ACPI_LPS0_EXIT_AMD:
+			return "lps0 exit";
+		}
+	}
+
+	return "unknown";
+}
+
 static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, guid_t dsm_guid)
 {
 	union acpi_object *out_obj;
@@ -331,13 +370,13 @@ static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, g
 					rev_id, func, NULL);
 	ACPI_FREE(out_obj);
 
-	acpi_handle_debug(lps0_device_handle, "_DSM function %u evaluation %s\n",
-			  func, out_obj ? "successful" : "failed");
-}
-
-static bool acpi_s2idle_vendor_amd(void)
-{
-	return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
+	lps0_dsm_state = func;
+	if (pm_debug_messages_on) {
+		acpi_handle_info(lps0_device_handle,
+				"%s transitioned to state %s\n",
+				 out_obj ? "Successfully" : "Failed to",
+				 acpi_sleep_dsm_state_to_str(lps0_dsm_state));
+	}
 }
 
 static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *dsm_guid)
@@ -487,9 +526,6 @@ int acpi_s2idle_prepare_late(void)
 	if (lps0_dsm_func_mask_microsoft > 0) {
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
 				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-		/* modern standby entry */
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
 	}
 
 	list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) {
@@ -513,6 +549,28 @@ void acpi_s2idle_check(void)
 	}
 }
 
+bool lps0_s2idle_wake(void)
+{
+	if (!lps0_device_handle || sleep_no_lps0)
+		goto out;
+
+	/* avoid running on the first go through the s2idle loop */
+	if (lps0_dsm_func_mask_microsoft > 0) {
+		int target;
+
+		if (lps0_dsm_state == ACPI_LPS0_ENTRY ||
+		    lps0_dsm_state == ACPI_LPS0_MS_EXIT)
+			target = ACPI_LPS0_MS_ENTRY;
+		else
+			target = ACPI_LPS0_MS_EXIT;
+		acpi_sleep_run_lps0_dsm(target,
+					lps0_dsm_func_mask_microsoft,
+					lps0_dsm_guid_microsoft);
+	}
+out:
+	return acpi_s2idle_wake();
+}
+
 void acpi_s2idle_restore_early(void)
 {
 	struct acpi_s2idle_dev_ops *handler;
@@ -524,11 +582,6 @@ void acpi_s2idle_restore_early(void)
 		if (handler->restore)
 			handler->restore();
 
-	/* Modern standby exit */
-	if (lps0_dsm_func_mask_microsoft > 0)
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
-				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-
 	/* LPS0 exit */
 	if (lps0_dsm_func_mask > 0)
 		acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ?
@@ -555,7 +608,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
 	.prepare = acpi_s2idle_prepare,
 	.prepare_late = acpi_s2idle_prepare_late,
 	.check = acpi_s2idle_check,
-	.wake = acpi_s2idle_wake,
+	.wake = lps0_s2idle_wake,
 	.restore_early = acpi_s2idle_restore_early,
 	.restore = acpi_s2idle_restore,
 	.end = acpi_s2idle_end,

base-commit: 7124d7671af0facf115d70f9d1fadde0d768d325
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
  2023-04-20 16:09 [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls Mario Limonciello
@ 2023-04-21  6:15 ` kernel test robot
  2023-04-24 12:20 ` kernel test robot
  2023-04-24 15:13 ` kernel test robot
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-04-21  6:15 UTC (permalink / raw)
  To: Mario Limonciello, rafael, linux-kernel
  Cc: oe-kbuild-all, linux-pm, Shyam-sundar.S-k, Mario Limonciello,
	Len Brown, linux-acpi

Hi Mario,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 7124d7671af0facf115d70f9d1fadde0d768d325]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-x86-Separate-out-the-Microsoft-_DSM-function-calls/20230421-001547
base:   7124d7671af0facf115d70f9d1fadde0d768d325
patch link:    https://lore.kernel.org/r/20230420160923.14127-1-mario.limonciello%40amd.com
patch subject: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230421/202304211425.SUKWdyf7-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Mario-Limonciello/ACPI-x86-Separate-out-the-Microsoft-_DSM-function-calls/20230421-001547
        git checkout e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304211425.SUKWdyf7-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/acpi/x86/s2idle.c:552:6: warning: no previous prototype for 'lps0_s2idle_wake' [-Wmissing-prototypes]
     552 | bool lps0_s2idle_wake(void)
         |      ^~~~~~~~~~~~~~~~


vim +/lps0_s2idle_wake +552 drivers/acpi/x86/s2idle.c

   551	
 > 552	bool lps0_s2idle_wake(void)
   553	{
   554		if (!lps0_device_handle || sleep_no_lps0)
   555			goto out;
   556	
   557		/* avoid running on the first go through the s2idle loop */
   558		if (lps0_dsm_func_mask_microsoft > 0) {
   559			int target;
   560	
   561			if (lps0_dsm_state == ACPI_LPS0_ENTRY ||
   562			    lps0_dsm_state == ACPI_LPS0_MS_EXIT)
   563				target = ACPI_LPS0_MS_ENTRY;
   564			else
   565				target = ACPI_LPS0_MS_EXIT;
   566			acpi_sleep_run_lps0_dsm(target,
   567						lps0_dsm_func_mask_microsoft,
   568						lps0_dsm_guid_microsoft);
   569		}
   570	out:
   571		return acpi_s2idle_wake();
   572	}
   573	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
  2023-04-20 16:09 [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls Mario Limonciello
  2023-04-21  6:15 ` kernel test robot
@ 2023-04-24 12:20 ` kernel test robot
  2023-04-24 15:13 ` kernel test robot
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-04-24 12:20 UTC (permalink / raw)
  To: Mario Limonciello, rafael, linux-kernel
  Cc: llvm, oe-kbuild-all, linux-pm, Shyam-sundar.S-k,
	Mario Limonciello, Len Brown, linux-acpi

Hi Mario,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 7124d7671af0facf115d70f9d1fadde0d768d325]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-x86-Separate-out-the-Microsoft-_DSM-function-calls/20230421-001547
base:   7124d7671af0facf115d70f9d1fadde0d768d325
patch link:    https://lore.kernel.org/r/20230420160923.14127-1-mario.limonciello%40amd.com
patch subject: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
config: i386-randconfig-a002-20230424 (https://download.01.org/0day-ci/archive/20230424/202304242025.fz8SDA8d-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
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/e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Mario-Limonciello/ACPI-x86-Separate-out-the-Microsoft-_DSM-function-calls/20230421-001547
        git checkout e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
        # 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 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/acpi/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304242025.fz8SDA8d-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/acpi/x86/s2idle.c:552:6: warning: no previous prototype for function 'lps0_s2idle_wake' [-Wmissing-prototypes]
   bool lps0_s2idle_wake(void)
        ^
   drivers/acpi/x86/s2idle.c:552:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool lps0_s2idle_wake(void)
   ^
   static 
   1 warning generated.


vim +/lps0_s2idle_wake +552 drivers/acpi/x86/s2idle.c

   551	
 > 552	bool lps0_s2idle_wake(void)
   553	{
   554		if (!lps0_device_handle || sleep_no_lps0)
   555			goto out;
   556	
   557		/* avoid running on the first go through the s2idle loop */
   558		if (lps0_dsm_func_mask_microsoft > 0) {
   559			int target;
   560	
   561			if (lps0_dsm_state == ACPI_LPS0_ENTRY ||
   562			    lps0_dsm_state == ACPI_LPS0_MS_EXIT)
   563				target = ACPI_LPS0_MS_ENTRY;
   564			else
   565				target = ACPI_LPS0_MS_EXIT;
   566			acpi_sleep_run_lps0_dsm(target,
   567						lps0_dsm_func_mask_microsoft,
   568						lps0_dsm_guid_microsoft);
   569		}
   570	out:
   571		return acpi_s2idle_wake();
   572	}
   573	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
  2023-04-20 16:09 [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls Mario Limonciello
  2023-04-21  6:15 ` kernel test robot
  2023-04-24 12:20 ` kernel test robot
@ 2023-04-24 15:13 ` kernel test robot
  2023-04-24 22:22   ` Limonciello, Mario
  2 siblings, 1 reply; 5+ messages in thread
From: kernel test robot @ 2023-04-24 15:13 UTC (permalink / raw)
  To: Mario Limonciello, rafael, linux-kernel
  Cc: oe-kbuild-all, linux-pm, Shyam-sundar.S-k, Mario Limonciello,
	Len Brown, linux-acpi

Hi Mario,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 7124d7671af0facf115d70f9d1fadde0d768d325]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-x86-Separate-out-the-Microsoft-_DSM-function-calls/20230421-001547
base:   7124d7671af0facf115d70f9d1fadde0d768d325
patch link:    https://lore.kernel.org/r/20230420160923.14127-1-mario.limonciello%40amd.com
patch subject: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
config: i386-randconfig-s003 (https://download.01.org/0day-ci/archive/20230424/202304242231.68KXGyif-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Mario-Limonciello/ACPI-x86-Separate-out-the-Microsoft-_DSM-function-calls/20230421-001547
        git checkout e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 olddefconfig
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/acpi/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304242231.68KXGyif-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/acpi/x86/s2idle.c:479:13: sparse: sparse: restricted suspend_state_t degrades to integer
   drivers/acpi/x86/s2idle.c:479:33: sparse: sparse: restricted suspend_state_t degrades to integer
>> drivers/acpi/x86/s2idle.c:552:6: sparse: sparse: symbol 'lps0_s2idle_wake' was not declared. Should it be static?

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
  2023-04-24 15:13 ` kernel test robot
@ 2023-04-24 22:22   ` Limonciello, Mario
  0 siblings, 0 replies; 5+ messages in thread
From: Limonciello, Mario @ 2023-04-24 22:22 UTC (permalink / raw)
  To: kernel test robot, rafael, linux-kernel
  Cc: oe-kbuild-all, linux-pm, S-k, Shyam-sundar, Len Brown, linux-acpi

[Public]



> -----Original Message-----
> From: kernel test robot <lkp@intel.com>
> Sent: Monday, April 24, 2023 10:14
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; rafael@kernel.org;
> linux-kernel@vger.kernel.org
> Cc: oe-kbuild-all@lists.linux.dev; linux-pm@vger.kernel.org; S-k, Shyam-sundar
> <Shyam-sundar.S-k@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Len Brown <lenb@kernel.org>; linux-
> acpi@vger.kernel.org
> Subject: Re: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls
> 
> Hi Mario,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on 7124d7671af0facf115d70f9d1fadde0d768d325]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-
> x86-Separate-out-the-Microsoft-_DSM-function-calls/20230421-001547
> base:   7124d7671af0facf115d70f9d1fadde0d768d325
> patch link:    https://lore.kernel.org/r/20230420160923.14127-1-
> mario.limonciello%40amd.com
> patch subject: [PATCH] ACPI: x86: Separate out the Microsoft _DSM function
> calls
> config: i386-randconfig-s003 (https://download.01.org/0day-
> ci/archive/20230424/202304242231.68KXGyif-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> reproduce:
>         # apt-get install sparse
>         # sparse version: v0.6.4-39-gce1a6720-dirty
>         # https://github.com/intel-lab-
> lkp/linux/commit/e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Mario-Limonciello/ACPI-x86-Separate-out-
> the-Microsoft-_DSM-function-calls/20230421-001547
>         git checkout e4ea0d2f15f2d0486bc3b4f59cbf9cea6c63fda1
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir
> ARCH=i386 olddefconfig
>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir
> ARCH=i386 SHELL=/bin/bash drivers/acpi/
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202304242231.68KXGyif-
> lkp@intel.com/
> 
> sparse warnings: (new ones prefixed by >>)
>    drivers/acpi/x86/s2idle.c:479:13: sparse: sparse: restricted suspend_state_t
> degrades to integer
>    drivers/acpi/x86/s2idle.c:479:33: sparse: sparse: restricted suspend_state_t
> degrades to integer
> >> drivers/acpi/x86/s2idle.c:552:6: sparse: sparse: symbol 'lps0_s2idle_wake'
> was not declared. Should it be static?
> 

Besides the problem caught by the robot I was looking at a BIOS debug log for Windows
recently and noticed the events come in a different order.

I re-reviewed the Microsoft spec and it makes it clear we've been doing it wrong in Linux too.

On the way down it should be
3->7->5
On the way back up it should be:
6->8->4

I need to redo all of my testing with this new assumption before I'm confident on such a change.
I'll include an assertion of confidence from that testing to help decide if this should actually wait
for 6.5.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-24 22:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-20 16:09 [PATCH] ACPI: x86: Separate out the Microsoft _DSM function calls Mario Limonciello
2023-04-21  6:15 ` kernel test robot
2023-04-24 12:20 ` kernel test robot
2023-04-24 15:13 ` kernel test robot
2023-04-24 22:22   ` Limonciello, Mario

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.