All uses in the kernel are currently already oriented around
suspend/resume. As some other parts of the kernel may also use these
messages in functions that could also be used outside of
suspend/resume, only enable in suspend/resume path.
Signed-off-by: Mario Limonciello <[email protected]>
---
v1->v2:
* Remove pointless do/while
* Add a function pm_debug_messages_should_print
* Export pm_debug_messages_should_print so modules can use it too.
---
include/linux/suspend.h | 20 +++++++++-----------
kernel/power/main.c | 6 ++++++
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 1a0426e6761c..9a3cc0b89208 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
#ifdef CONFIG_PM_SLEEP_DEBUG
extern bool pm_print_times_enabled;
extern bool pm_debug_messages_on;
+extern bool pm_debug_messages_should_print(void);
static inline int pm_dyn_debug_messages_on(void)
{
#ifdef CONFIG_DYNAMIC_DEBUG
@@ -567,17 +568,13 @@ static inline int pm_dyn_debug_messages_on(void)
#define pr_fmt(fmt) "PM: " fmt
#endif
#define __pm_pr_dbg(fmt, ...) \
- do { \
- if (pm_debug_messages_on) \
- printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); \
- else if (pm_dyn_debug_messages_on()) \
- pr_debug(fmt, ##__VA_ARGS__); \
- } while (0)
+ if (pm_debug_messages_should_print()) \
+ printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); \
+ else if (pm_dyn_debug_messages_on()) \
+ pr_debug(fmt, ##__VA_ARGS__);
#define __pm_deferred_pr_dbg(fmt, ...) \
- do { \
- if (pm_debug_messages_on) \
- printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); \
- } while (0)
+ if (pm_debug_messages_should_print()) \
+ printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);
#else
#define pm_print_times_enabled (false)
#define pm_debug_messages_on (false)
@@ -593,7 +590,8 @@ static inline int pm_dyn_debug_messages_on(void)
/**
* pm_pr_dbg - print pm sleep debug messages
*
- * If pm_debug_messages_on is enabled, print message.
+ * If pm_debug_messages_on is enabled and the system is entering/leaving
+ * suspend, print message.
* If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
* print message only from instances explicitly enabled on dynamic debug's
* control.
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3113ec2f1db4..daa535012e51 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
bool pm_debug_messages_on __read_mostly;
+bool pm_debug_messages_should_print(void)
+{
+ return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;
+}
+EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
+
static ssize_t pm_debug_messages_show(struct kobject *kobj,
struct kobj_attribute *attr, char *buf)
{
base-commit: 7736c431466abb54a2679dc257f739fddfa84295
--
2.34.1
Enabling debugging messages for the state requires turning on dynamic
debugging for the file. To make it more accessible, use
`pm_debug_messages` and clearer strings for what is happening.
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/acpi/x86/s2idle.c | 52 ++++++++++++++++++++++++++++++++++-----
1 file changed, 46 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index e499c60c4579..7681f6ecab67 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,14 +370,15 @@ 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");
+ 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 bool acpi_s2idle_vendor_amd(void)
-{
- return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
-}
static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *dsm_guid)
{
--
2.34.1
Using pm_pr_dbg() allows users to toggle `/sys/power/pm_debug_messages`
as a single knob to turn on messages that amd-pmc can emit to aid in
any s2idle debugging.
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/platform/x86/amd/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 427905714f79..1304cd6f13f6 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -543,7 +543,7 @@ static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
}
if (dev)
- dev_dbg(pdev->dev, "SMU idlemask s0i3: 0x%x\n", val);
+ pm_pr_dbg("SMU idlemask s0i3: 0x%x\n", val);
if (s)
seq_printf(s, "SMU idlemask : 0x%x\n", val);
@@ -769,7 +769,7 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
*arg |= (duration << 16);
rc = rtc_alarm_irq_enable(rtc_device, 0);
- dev_dbg(pdev->dev, "wakeup timer programmed for %lld seconds\n", duration);
+ pm_pr_dbg("wakeup timer programmed for %lld seconds\n", duration);
return rc;
}
--
2.34.1
Thu, Jun 01, 2023 at 06:29:20PM -0500, Mario Limonciello kirjoitti:
> All uses in the kernel are currently already oriented around
> suspend/resume. As some other parts of the kernel may also use these
> messages in functions that could also be used outside of
> suspend/resume, only enable in suspend/resume path.
...
> * Remove pointless do/while
I believe this is a joke.
This guards against weirdness, like
__pm_pr_dbg(foo)
else
bar;
NAK for this particular change.
--
With Best Regards,
Andy Shevchenko
Hi Mario,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 7736c431466abb54a2679dc257f739fddfa84295]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-x86-Add-pm_debug_messages-for-LPS0-_DSM-state-tracking/20230602-073044
base: 7736c431466abb54a2679dc257f739fddfa84295
patch link: https://lore.kernel.org/r/20230601232923.1248-1-mario.limonciello%40amd.com
patch subject: [PATCH v3 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
config: powerpc-randconfig-r035-20230531 (https://download.01.org/0day-ci/archive/20230602/[email protected]/config)
compiler: powerpc-linux-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
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/5828d770a5f17c4028520050068fd3cdd13b80a1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/ACPI-x86-Add-pm_debug_messages-for-LPS0-_DSM-state-tracking/20230602-073044
git checkout 5828d770a5f17c4028520050068fd3cdd13b80a1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/base/power/ kernel/power/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/base/power/wakeup.c: In function 'pm_print_active_wakeup_sources':
>> drivers/base/power/wakeup.c:861:12: warning: suggest explicit braces to avoid ambiguous 'else' [-Wdangling-else]
861 | if (!active && last_activity_ws)
| ^
--
kernel/power/hibernate.c: In function 'disk_store':
>> kernel/power/hibernate.c:1158:12: warning: suggest explicit braces to avoid ambiguous 'else' [-Wdangling-else]
1158 | if (!error)
| ^
vim +/else +861 drivers/base/power/wakeup.c
074037ec79bea7 Rafael J. Wysocki 2010-09-22 841
bb177fedd348c9 Julius Werner 2013-06-12 842 void pm_print_active_wakeup_sources(void)
a938da0682c248 Todd Poynor 2012-08-12 843 {
a938da0682c248 Todd Poynor 2012-08-12 844 struct wakeup_source *ws;
ea0212f40c6bc0 Thomas Gleixner 2017-06-25 845 int srcuidx, active = 0;
a938da0682c248 Todd Poynor 2012-08-12 846 struct wakeup_source *last_activity_ws = NULL;
a938da0682c248 Todd Poynor 2012-08-12 847
ea0212f40c6bc0 Thomas Gleixner 2017-06-25 848 srcuidx = srcu_read_lock(&wakeup_srcu);
2591e7b17c0d3f Madhuparna Bhowmik 2020-03-04 849 list_for_each_entry_rcu_locked(ws, &wakeup_sources, entry) {
a938da0682c248 Todd Poynor 2012-08-12 850 if (ws->active) {
74a1dd86d1739e Stephen Boyd 2019-03-25 851 pm_pr_dbg("active wakeup source: %s\n", ws->name);
a938da0682c248 Todd Poynor 2012-08-12 852 active = 1;
a938da0682c248 Todd Poynor 2012-08-12 853 } else if (!active &&
a938da0682c248 Todd Poynor 2012-08-12 854 (!last_activity_ws ||
a938da0682c248 Todd Poynor 2012-08-12 855 ktime_to_ns(ws->last_time) >
a938da0682c248 Todd Poynor 2012-08-12 856 ktime_to_ns(last_activity_ws->last_time))) {
a938da0682c248 Todd Poynor 2012-08-12 857 last_activity_ws = ws;
a938da0682c248 Todd Poynor 2012-08-12 858 }
a938da0682c248 Todd Poynor 2012-08-12 859 }
a938da0682c248 Todd Poynor 2012-08-12 860
a938da0682c248 Todd Poynor 2012-08-12 @861 if (!active && last_activity_ws)
74a1dd86d1739e Stephen Boyd 2019-03-25 862 pm_pr_dbg("last active wakeup source: %s\n",
a938da0682c248 Todd Poynor 2012-08-12 863 last_activity_ws->name);
ea0212f40c6bc0 Thomas Gleixner 2017-06-25 864 srcu_read_unlock(&wakeup_srcu, srcuidx);
a938da0682c248 Todd Poynor 2012-08-12 865 }
bb177fedd348c9 Julius Werner 2013-06-12 866 EXPORT_SYMBOL_GPL(pm_print_active_wakeup_sources);
a938da0682c248 Todd Poynor 2012-08-12 867
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki