* [PATCH v8 0/4] Add vendor agnostic mechanism to report hardware sleep
@ 2023-04-12 19:49 Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 2/4] platform/x86/amd: pmc: Report duration of time in hw sleep state Mario Limonciello
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Mario Limonciello @ 2023-04-12 19:49 UTC (permalink / raw)
To: Box David E, jstultz, pavel, svenva, platform-driver-x86, linux-pm
Cc: Shyam-sundar.S-k, rrangel, Jain Rajat, hdegoede,
Mario Limonciello, linux-kernel
An important part of validating that s0ix worked properly is to check how
much of a cycle was spent in a hardware sleep state.
The reporting of hardware sleep is a mix of kernel messages and sysfs
files that vary from vendor to vendor. Collecting this information
requires extra information on the kernel command line or fetching from
debugfs.
To make this information more readily accessible introduce a new file in
suspend_stats that drivers can report into during their resume routine.
Userspace can fetch this information and compare it against the duration
of the cycle to allow determining residency percentages and flagging
problems.
Mario Limonciello (4):
PM: Add sysfs files to represent time spent in hardware sleep state
platform/x86/amd: pmc: Report duration of time in hw sleep state
platform/x86/intel/pmc: core: Always capture counters on suspend
platform/x86/intel/pmc: core: Report duration of time in HW sleep
state
Documentation/ABI/testing/sysfs-power | 29 +++++++++++++
drivers/platform/x86/amd/pmc.c | 6 +--
drivers/platform/x86/intel/pmc/core.c | 16 ++++----
drivers/platform/x86/intel/pmc/core.h | 2 -
include/linux/suspend.h | 8 ++++
kernel/power/main.c | 59 +++++++++++++++++++++------
6 files changed, 95 insertions(+), 25 deletions(-)
base-commit: 09a9639e56c01c7a00d6c0ca63f4c7c41abe075d
--
2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v8 2/4] platform/x86/amd: pmc: Report duration of time in hw sleep state
2023-04-12 19:49 [PATCH v8 0/4] Add vendor agnostic mechanism to report hardware sleep Mario Limonciello
@ 2023-04-12 19:49 ` Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 3/4] platform/x86/intel/pmc: core: Always capture counters on suspend Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state Mario Limonciello
2 siblings, 0 replies; 9+ messages in thread
From: Mario Limonciello @ 2023-04-12 19:49 UTC (permalink / raw)
To: Box David E, jstultz, pavel, svenva, Shyam Sundar S K
Cc: rrangel, Jain Rajat, hdegoede, Mario Limonciello, Mark Gross,
platform-driver-x86, linux-kernel
amd_pmc displays a warning when a suspend didn't get to the deepest
state and a dynamic debugging message with the duration if it did.
Rather than logging to dynamic debugging the duration spent in the
deepest state, report this to the standard kernel reporting
infrastructure so that userspace software can query after the
suspend cycle is done.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v7->v8:
* Report max hw sleep time as well.
---
drivers/platform/x86/amd/pmc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 2edaae04a691..e610457136e6 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -393,9 +393,8 @@ static void amd_pmc_validate_deepest(struct amd_pmc_dev *pdev)
if (!table.s0i3_last_entry_status)
dev_warn(pdev->dev, "Last suspend didn't reach deepest state\n");
- else
- dev_dbg(pdev->dev, "Last suspend in deepest state for %lluus\n",
- table.timein_s0i3_lastcapture);
+ pm_report_hw_sleep_time(table.s0i3_last_entry_status ?
+ table.timein_s0i3_lastcapture : 0);
}
static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
@@ -1015,6 +1014,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
}
amd_pmc_dbgfs_register(dev);
+ pm_report_max_hw_sleep(U64_MAX);
return 0;
err_pci_dev_put:
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 3/4] platform/x86/intel/pmc: core: Always capture counters on suspend
2023-04-12 19:49 [PATCH v8 0/4] Add vendor agnostic mechanism to report hardware sleep Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 2/4] platform/x86/amd: pmc: Report duration of time in hw sleep state Mario Limonciello
@ 2023-04-12 19:49 ` Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state Mario Limonciello
2 siblings, 0 replies; 9+ messages in thread
From: Mario Limonciello @ 2023-04-12 19:49 UTC (permalink / raw)
To: Box David E, jstultz, pavel, svenva, Rajneesh Bhardwaj
Cc: Shyam-sundar.S-k, rrangel, Jain Rajat, hdegoede,
Mario Limonciello, David E . Box, Mark Gross,
platform-driver-x86, linux-kernel
Currently counters are only captured during suspend when the
warn_on_s0ix_failures module parameter is set.
In order to relay this counter information to the kernel reporting
infrastructure adjust it so that the counters are always captured.
warn_on_s0ix_failures will be utilized solely for messaging by
the driver instead.
Reviewed-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v5->v6:
* Pick up tag
v4->v5:
* Squash patches together
* Add extra pm_suspend_via_firmware() check for resume routine too
---
drivers/platform/x86/intel/pmc/core.c | 13 +++++--------
drivers/platform/x86/intel/pmc/core.h | 2 --
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index b9591969e0fa..925c5d676a43 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -1179,12 +1179,6 @@ static __maybe_unused int pmc_core_suspend(struct device *dev)
{
struct pmc_dev *pmcdev = dev_get_drvdata(dev);
- pmcdev->check_counters = false;
-
- /* No warnings on S0ix failures */
- if (!warn_on_s0ix_failures)
- return 0;
-
/* Check if the syspend will actually use S0ix */
if (pm_suspend_via_firmware())
return 0;
@@ -1197,7 +1191,6 @@ static __maybe_unused int pmc_core_suspend(struct device *dev)
if (pmc_core_dev_state_get(pmcdev, &pmcdev->s0ix_counter))
return -EIO;
- pmcdev->check_counters = true;
return 0;
}
@@ -1233,12 +1226,16 @@ static __maybe_unused int pmc_core_resume(struct device *dev)
const struct pmc_bit_map **maps = pmcdev->map->lpm_sts;
int offset = pmcdev->map->lpm_status_offset;
- if (!pmcdev->check_counters)
+ /* Check if the syspend used S0ix */
+ if (pm_suspend_via_firmware())
return 0;
if (!pmc_core_is_s0ix_failed(pmcdev))
return 0;
+ if (!warn_on_s0ix_failures)
+ return 0;
+
if (pmc_core_is_pc10_failed(pmcdev)) {
/* S0ix failed because of PC10 entry failure */
dev_info(dev, "CPU did not enter PC10!!! (PC10 cnt=0x%llx)\n",
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index 810204d758ab..51d73efceaf3 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -319,7 +319,6 @@ struct pmc_reg_map {
* @pmc_xram_read_bit: flag to indicate whether PMC XRAM shadow registers
* used to read MPHY PG and PLL status are available
* @mutex_lock: mutex to complete one transcation
- * @check_counters: On resume, check if counters are getting incremented
* @pc10_counter: PC10 residency counter
* @s0ix_counter: S0ix residency (step adjusted)
* @num_lpm_modes: Count of enabled modes
@@ -338,7 +337,6 @@ struct pmc_dev {
int pmc_xram_read_bit;
struct mutex lock; /* generic mutex lock for PMC Core */
- bool check_counters; /* Check for counter increments on resume */
u64 pc10_counter;
u64 s0ix_counter;
int num_lpm_modes;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
2023-04-12 19:49 [PATCH v8 0/4] Add vendor agnostic mechanism to report hardware sleep Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 2/4] platform/x86/amd: pmc: Report duration of time in hw sleep state Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 3/4] platform/x86/intel/pmc: core: Always capture counters on suspend Mario Limonciello
@ 2023-04-12 19:49 ` Mario Limonciello
2023-04-13 1:50 ` kernel test robot
` (2 more replies)
2 siblings, 3 replies; 9+ messages in thread
From: Mario Limonciello @ 2023-04-12 19:49 UTC (permalink / raw)
To: Box David E, jstultz, pavel, svenva, Rajneesh Bhardwaj
Cc: Shyam-sundar.S-k, rrangel, Jain Rajat, hdegoede,
Mario Limonciello, Mark Gross, platform-driver-x86, linux-kernel
intel_pmc_core displays a warning when the module parameter
`warn_on_s0ix_failures` is set and a suspend didn't get to a HW sleep
state.
Report this to the standard kernel reporting infrastructure so that
userspace software can query after the suspend cycle is done.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v7->v8:
* Report max sleep as well
---
drivers/platform/x86/intel/pmc/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 925c5d676a43..f9677104353d 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -1153,6 +1153,7 @@ static int pmc_core_probe(struct platform_device *pdev)
pmc_core_do_dmi_quirks(pmcdev);
pmc_core_dbgfs_register(pmcdev);
+ pm_report_max_hw_sleep(((1UL << 32) - 1) * pmc_core_adjust_slp_s0_step(pmcdev, 1));
device_initialized = true;
dev_info(&pdev->dev, " initialized\n");
@@ -1214,6 +1215,8 @@ static inline bool pmc_core_is_s0ix_failed(struct pmc_dev *pmcdev)
if (pmc_core_dev_state_get(pmcdev, &s0ix_counter))
return false;
+ pm_report_hw_sleep_time((u32)(s0ix_counter - pmcdev->s0ix_counter));
+
if (s0ix_counter == pmcdev->s0ix_counter)
return true;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
2023-04-12 19:49 ` [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state Mario Limonciello
@ 2023-04-13 1:50 ` kernel test robot
2023-04-13 9:23 ` Ilpo Järvinen
2023-04-14 1:58 ` kernel test robot
2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-04-13 1:50 UTC (permalink / raw)
To: Mario Limonciello, Box David E, jstultz, pavel, svenva,
Rajneesh Bhardwaj
Cc: llvm, oe-kbuild-all, Shyam-sundar.S-k, rrangel, Jain Rajat,
hdegoede, Mario Limonciello, Mark Gross, platform-driver-x86,
linux-kernel
Hi Mario,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 09a9639e56c01c7a00d6c0ca63f4c7c41abe075d]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-Add-sysfs-files-to-represent-time-spent-in-hardware-sleep-state/20230413-035220
base: 09a9639e56c01c7a00d6c0ca63f4c7c41abe075d
patch link: https://lore.kernel.org/r/20230412194917.7164-5-mario.limonciello%40amd.com
patch subject: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
config: i386-randconfig-a004-20230410 (https://download.01.org/0day-ci/archive/20230413/202304130908.LOiMWRYR-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/315b1dd23cbedfd2848c8ac8ec1f77c3610b955e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/PM-Add-sysfs-files-to-represent-time-spent-in-hardware-sleep-state/20230413-035220
git checkout 315b1dd23cbedfd2848c8ac8ec1f77c3610b955e
# 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/platform/x86/intel/pmc/
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/202304130908.LOiMWRYR-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/platform/x86/intel/pmc/core.c:1156:31: warning: shift count >= width of type [-Wshift-count-overflow]
pm_report_max_hw_sleep(((1UL << 32) - 1) * pmc_core_adjust_slp_s0_step(pmcdev, 1));
^ ~~
1 warning generated.
vim +1156 drivers/platform/x86/intel/pmc/core.c
1097
1098 static int pmc_core_probe(struct platform_device *pdev)
1099 {
1100 static bool device_initialized;
1101 struct pmc_dev *pmcdev;
1102 const struct x86_cpu_id *cpu_id;
1103 void (*core_init)(struct pmc_dev *pmcdev);
1104 u64 slp_s0_addr;
1105
1106 if (device_initialized)
1107 return -ENODEV;
1108
1109 pmcdev = devm_kzalloc(&pdev->dev, sizeof(*pmcdev), GFP_KERNEL);
1110 if (!pmcdev)
1111 return -ENOMEM;
1112
1113 platform_set_drvdata(pdev, pmcdev);
1114 pmcdev->pdev = pdev;
1115
1116 cpu_id = x86_match_cpu(intel_pmc_core_ids);
1117 if (!cpu_id)
1118 return -ENODEV;
1119
1120 core_init = (void (*)(struct pmc_dev *))cpu_id->driver_data;
1121
1122 /*
1123 * Coffee Lake has CPU ID of Kaby Lake and Cannon Lake PCH. So here
1124 * Sunrisepoint PCH regmap can't be used. Use Cannon Lake PCH regmap
1125 * in this case.
1126 */
1127 if (core_init == spt_core_init && !pci_dev_present(pmc_pci_ids))
1128 core_init = cnp_core_init;
1129
1130 mutex_init(&pmcdev->lock);
1131 core_init(pmcdev);
1132
1133
1134 if (lpit_read_residency_count_address(&slp_s0_addr)) {
1135 pmcdev->base_addr = PMC_BASE_ADDR_DEFAULT;
1136
1137 if (page_is_ram(PHYS_PFN(pmcdev->base_addr)))
1138 return -ENODEV;
1139 } else {
1140 pmcdev->base_addr = slp_s0_addr - pmcdev->map->slp_s0_offset;
1141 }
1142
1143 pmcdev->regbase = ioremap(pmcdev->base_addr,
1144 pmcdev->map->regmap_length);
1145 if (!pmcdev->regbase)
1146 return -ENOMEM;
1147
1148 if (pmcdev->core_configure)
1149 pmcdev->core_configure(pmcdev);
1150
1151 pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit(pmcdev);
1152 pmc_core_get_low_power_modes(pdev);
1153 pmc_core_do_dmi_quirks(pmcdev);
1154
1155 pmc_core_dbgfs_register(pmcdev);
> 1156 pm_report_max_hw_sleep(((1UL << 32) - 1) * pmc_core_adjust_slp_s0_step(pmcdev, 1));
1157
1158 device_initialized = true;
1159 dev_info(&pdev->dev, " initialized\n");
1160
1161 return 0;
1162 }
1163
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
2023-04-12 19:49 ` [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state Mario Limonciello
2023-04-13 1:50 ` kernel test robot
@ 2023-04-13 9:23 ` Ilpo Järvinen
2023-04-13 22:40 ` Limonciello, Mario
2023-04-14 1:58 ` kernel test robot
2 siblings, 1 reply; 9+ messages in thread
From: Ilpo Järvinen @ 2023-04-13 9:23 UTC (permalink / raw)
To: Mario Limonciello
Cc: Box David E, jstultz, pavel, svenva, Rajneesh Bhardwaj,
Shyam-sundar.S-k, rrangel, Jain Rajat, hdegoede, Mark Gross,
platform-driver-x86, LKML
On Wed, 12 Apr 2023, Mario Limonciello wrote:
> intel_pmc_core displays a warning when the module parameter
> `warn_on_s0ix_failures` is set and a suspend didn't get to a HW sleep
> state.
>
> Report this to the standard kernel reporting infrastructure so that
> userspace software can query after the suspend cycle is done.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v7->v8:
> * Report max sleep as well
> ---
> drivers/platform/x86/intel/pmc/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 925c5d676a43..f9677104353d 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1153,6 +1153,7 @@ static int pmc_core_probe(struct platform_device *pdev)
> pmc_core_do_dmi_quirks(pmcdev);
>
> pmc_core_dbgfs_register(pmcdev);
> + pm_report_max_hw_sleep(((1UL << 32) - 1) * pmc_core_adjust_slp_s0_step(pmcdev, 1));
Technically this is FIELD_MAX(SLP_S0_RES_COUNTER_MASK) * pmc_core_adjust...?
Where the define is:
#define SLP_S0_RES_COUNTER_MASK GENMASK(31, 0)
>
> device_initialized = true;
> dev_info(&pdev->dev, " initialized\n");
> @@ -1214,6 +1215,8 @@ static inline bool pmc_core_is_s0ix_failed(struct pmc_dev *pmcdev)
> if (pmc_core_dev_state_get(pmcdev, &s0ix_counter))
> return false;
>
> + pm_report_hw_sleep_time((u32)(s0ix_counter - pmcdev->s0ix_counter));
> +
> if (s0ix_counter == pmcdev->s0ix_counter)
> return true;
>
>
--
i.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
2023-04-13 9:23 ` Ilpo Järvinen
@ 2023-04-13 22:40 ` Limonciello, Mario
2023-04-14 0:35 ` David E. Box
0 siblings, 1 reply; 9+ messages in thread
From: Limonciello, Mario @ 2023-04-13 22:40 UTC (permalink / raw)
To: Ilpo Järvinen, Box David E
Cc: jstultz, pavel, svenva, Rajneesh Bhardwaj, S-k, Shyam-sundar,
rrangel, Jain Rajat, hdegoede, Mark Gross, platform-driver-x86,
LKML
[Public]
> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Thursday, April 13, 2023 04:24
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Box David E <david.e.box@intel.com>; jstultz@google.com;
> pavel@ucw.cz; svenva@chromium.org; Rajneesh Bhardwaj
> <irenic.rajneesh@gmail.com>; S-k, Shyam-sundar <Shyam-sundar.S-
> k@amd.com>; rrangel@chromium.org; Jain Rajat <rajatja@google.com>;
> hdegoede@redhat.com; Mark Gross <markgross@kernel.org>; platform-
> driver-x86@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of
> time in HW sleep state
>
> On Wed, 12 Apr 2023, Mario Limonciello wrote:
>
> > intel_pmc_core displays a warning when the module parameter
> > `warn_on_s0ix_failures` is set and a suspend didn't get to a HW sleep
> > state.
> >
> > Report this to the standard kernel reporting infrastructure so that
> > userspace software can query after the suspend cycle is done.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> > v7->v8:
> > * Report max sleep as well
> > ---
> > drivers/platform/x86/intel/pmc/core.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/platform/x86/intel/pmc/core.c
> b/drivers/platform/x86/intel/pmc/core.c
> > index 925c5d676a43..f9677104353d 100644
> > --- a/drivers/platform/x86/intel/pmc/core.c
> > +++ b/drivers/platform/x86/intel/pmc/core.c
> > @@ -1153,6 +1153,7 @@ static int pmc_core_probe(struct platform_device
> *pdev)
> > pmc_core_do_dmi_quirks(pmcdev);
> >
> > pmc_core_dbgfs_register(pmcdev);
> > + pm_report_max_hw_sleep(((1UL << 32) - 1) *
> pmc_core_adjust_slp_s0_step(pmcdev, 1));
>
> Technically this is FIELD_MAX(SLP_S0_RES_COUNTER_MASK) *
> pmc_core_adjust...?
> Where the define is:
> #define SLP_S0_RES_COUNTER_MASK GENMASK(31, 0)
That's fine by me to switch it over, it certainly makes it a lot more readable.
I took the value from @Box David E to use suggested in v7, so what are your
thoughts?
The current version has an overflow error reported by the robot for i386, so it
definitely needs some sort of change.
>
> >
> > device_initialized = true;
> > dev_info(&pdev->dev, " initialized\n");
> > @@ -1214,6 +1215,8 @@ static inline bool pmc_core_is_s0ix_failed(struct
> pmc_dev *pmcdev)
> > if (pmc_core_dev_state_get(pmcdev, &s0ix_counter))
> > return false;
> >
> > + pm_report_hw_sleep_time((u32)(s0ix_counter - pmcdev-
> >s0ix_counter));
> > +
> > if (s0ix_counter == pmcdev->s0ix_counter)
> > return true;
> >
> >
>
> --
> i.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
2023-04-13 22:40 ` Limonciello, Mario
@ 2023-04-14 0:35 ` David E. Box
0 siblings, 0 replies; 9+ messages in thread
From: David E. Box @ 2023-04-14 0:35 UTC (permalink / raw)
To: Limonciello, Mario, Ilpo Järvinen, Box David E
Cc: jstultz, pavel, svenva, Rajneesh Bhardwaj, S-k, Shyam-sundar,
rrangel, Jain Rajat, hdegoede, Mark Gross, platform-driver-x86,
LKML
On Thu, 2023-04-13 at 22:40 +0000, Limonciello, Mario wrote:
> [Public]
>
>
>
> > -----Original Message-----
> > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Sent: Thursday, April 13, 2023 04:24
> > To: Limonciello, Mario <Mario.Limonciello@amd.com>
> > Cc: Box David E <david.e.box@intel.com>; jstultz@google.com;
> > pavel@ucw.cz; svenva@chromium.org; Rajneesh Bhardwaj
> > <irenic.rajneesh@gmail.com>; S-k, Shyam-sundar <Shyam-sundar.S-
> > k@amd.com>; rrangel@chromium.org; Jain Rajat <rajatja@google.com>;
> > hdegoede@redhat.com; Mark Gross <markgross@kernel.org>; platform-
> > driver-x86@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of
> > time in HW sleep state
> >
> > On Wed, 12 Apr 2023, Mario Limonciello wrote:
> >
> > > intel_pmc_core displays a warning when the module parameter
> > > `warn_on_s0ix_failures` is set and a suspend didn't get to a HW sleep
> > > state.
> > >
> > > Report this to the standard kernel reporting infrastructure so that
> > > userspace software can query after the suspend cycle is done.
> > >
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > > v7->v8:
> > > * Report max sleep as well
> > > ---
> > > drivers/platform/x86/intel/pmc/core.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/platform/x86/intel/pmc/core.c
> > b/drivers/platform/x86/intel/pmc/core.c
> > > index 925c5d676a43..f9677104353d 100644
> > > --- a/drivers/platform/x86/intel/pmc/core.c
> > > +++ b/drivers/platform/x86/intel/pmc/core.c
> > > @@ -1153,6 +1153,7 @@ static int pmc_core_probe(struct platform_device
> > *pdev)
> > > pmc_core_do_dmi_quirks(pmcdev);
> > >
> > > pmc_core_dbgfs_register(pmcdev);
> > > + pm_report_max_hw_sleep(((1UL << 32) - 1) *
> > pmc_core_adjust_slp_s0_step(pmcdev, 1));
> >
> > Technically this is FIELD_MAX(SLP_S0_RES_COUNTER_MASK) *
> > pmc_core_adjust...?
> > Where the define is:
> > #define SLP_S0_RES_COUNTER_MASK GENMASK(31, 0)
>
> That's fine by me to switch it over, it certainly makes it a lot more
> readable.
> I took the value from @Box David E to use suggested in v7, so what are your
> thoughts?
Ilpo's suggestion is preferable. The warning comes from using 1UL, long being 4
bytes on i386.
>
> The current version has an overflow error reported by the robot for i386, so
> it
> definitely needs some sort of change.
Resolved by using the macro. With Ilpo's suggestion you can add my reviewed by.
Thanks.
David
>
> >
> > >
> > > device_initialized = true;
> > > dev_info(&pdev->dev, " initialized\n");
> > > @@ -1214,6 +1215,8 @@ static inline bool pmc_core_is_s0ix_failed(struct
> > pmc_dev *pmcdev)
> > > if (pmc_core_dev_state_get(pmcdev, &s0ix_counter))
> > > return false;
> > >
> > > + pm_report_hw_sleep_time((u32)(s0ix_counter - pmcdev-
> > > s0ix_counter));
> > > +
> > > if (s0ix_counter == pmcdev->s0ix_counter)
> > > return true;
> > >
> > >
> >
> > --
> > i.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
2023-04-12 19:49 ` [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state Mario Limonciello
2023-04-13 1:50 ` kernel test robot
2023-04-13 9:23 ` Ilpo Järvinen
@ 2023-04-14 1:58 ` kernel test robot
2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-04-14 1:58 UTC (permalink / raw)
To: Mario Limonciello, Box David E, jstultz, pavel, svenva,
Rajneesh Bhardwaj
Cc: oe-kbuild-all, Shyam-sundar.S-k, rrangel, Jain Rajat, hdegoede,
Mario Limonciello, Mark Gross, platform-driver-x86, linux-kernel
Hi Mario,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 09a9639e56c01c7a00d6c0ca63f4c7c41abe075d]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-Add-sysfs-files-to-represent-time-spent-in-hardware-sleep-state/20230413-035220
base: 09a9639e56c01c7a00d6c0ca63f4c7c41abe075d
patch link: https://lore.kernel.org/r/20230412194917.7164-5-mario.limonciello%40amd.com
patch subject: [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20230414/202304140957.hkvWzLzM-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/315b1dd23cbedfd2848c8ac8ec1f77c3610b955e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/PM-Add-sysfs-files-to-represent-time-spent-in-hardware-sleep-state/20230413-035220
git checkout 315b1dd23cbedfd2848c8ac8ec1f77c3610b955e
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/platform/x86/intel/pmc/
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/202304140957.hkvWzLzM-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/platform/x86/intel/pmc/core.c: In function 'pmc_core_probe':
>> drivers/platform/x86/intel/pmc/core.c:1156:38: warning: left shift count >= width of type [-Wshift-count-overflow]
1156 | pm_report_max_hw_sleep(((1UL << 32) - 1) * pmc_core_adjust_slp_s0_step(pmcdev, 1));
| ^~
vim +1156 drivers/platform/x86/intel/pmc/core.c
1097
1098 static int pmc_core_probe(struct platform_device *pdev)
1099 {
1100 static bool device_initialized;
1101 struct pmc_dev *pmcdev;
1102 const struct x86_cpu_id *cpu_id;
1103 void (*core_init)(struct pmc_dev *pmcdev);
1104 u64 slp_s0_addr;
1105
1106 if (device_initialized)
1107 return -ENODEV;
1108
1109 pmcdev = devm_kzalloc(&pdev->dev, sizeof(*pmcdev), GFP_KERNEL);
1110 if (!pmcdev)
1111 return -ENOMEM;
1112
1113 platform_set_drvdata(pdev, pmcdev);
1114 pmcdev->pdev = pdev;
1115
1116 cpu_id = x86_match_cpu(intel_pmc_core_ids);
1117 if (!cpu_id)
1118 return -ENODEV;
1119
1120 core_init = (void (*)(struct pmc_dev *))cpu_id->driver_data;
1121
1122 /*
1123 * Coffee Lake has CPU ID of Kaby Lake and Cannon Lake PCH. So here
1124 * Sunrisepoint PCH regmap can't be used. Use Cannon Lake PCH regmap
1125 * in this case.
1126 */
1127 if (core_init == spt_core_init && !pci_dev_present(pmc_pci_ids))
1128 core_init = cnp_core_init;
1129
1130 mutex_init(&pmcdev->lock);
1131 core_init(pmcdev);
1132
1133
1134 if (lpit_read_residency_count_address(&slp_s0_addr)) {
1135 pmcdev->base_addr = PMC_BASE_ADDR_DEFAULT;
1136
1137 if (page_is_ram(PHYS_PFN(pmcdev->base_addr)))
1138 return -ENODEV;
1139 } else {
1140 pmcdev->base_addr = slp_s0_addr - pmcdev->map->slp_s0_offset;
1141 }
1142
1143 pmcdev->regbase = ioremap(pmcdev->base_addr,
1144 pmcdev->map->regmap_length);
1145 if (!pmcdev->regbase)
1146 return -ENOMEM;
1147
1148 if (pmcdev->core_configure)
1149 pmcdev->core_configure(pmcdev);
1150
1151 pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit(pmcdev);
1152 pmc_core_get_low_power_modes(pdev);
1153 pmc_core_do_dmi_quirks(pmcdev);
1154
1155 pmc_core_dbgfs_register(pmcdev);
> 1156 pm_report_max_hw_sleep(((1UL << 32) - 1) * pmc_core_adjust_slp_s0_step(pmcdev, 1));
1157
1158 device_initialized = true;
1159 dev_info(&pdev->dev, " initialized\n");
1160
1161 return 0;
1162 }
1163
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-04-14 1:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-12 19:49 [PATCH v8 0/4] Add vendor agnostic mechanism to report hardware sleep Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 2/4] platform/x86/amd: pmc: Report duration of time in hw sleep state Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 3/4] platform/x86/intel/pmc: core: Always capture counters on suspend Mario Limonciello
2023-04-12 19:49 ` [PATCH v8 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state Mario Limonciello
2023-04-13 1:50 ` kernel test robot
2023-04-13 9:23 ` Ilpo Järvinen
2023-04-13 22:40 ` Limonciello, Mario
2023-04-14 0:35 ` David E. Box
2023-04-14 1:58 ` kernel test robot
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).