From: "Limonciello, Mario" <mario.limonciello@amd.com>
To: "Box, David E" <david.e.box@intel.com>,
"rafael@kernel.org" <rafael@kernel.org>
Cc: "jstultz@google.com" <jstultz@google.com>,
"Shyam-sundar.S-k@amd.com" <Shyam-sundar.S-k@amd.com>,
"markgross@kernel.org" <markgross@kernel.org>,
"rrangel@chromium.org" <rrangel@chromium.org>,
"Jain, Rajat" <rajatja@google.com>,
"irenic.rajneesh@gmail.com" <irenic.rajneesh@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"hdegoede@redhat.com" <hdegoede@redhat.com>,
"svenva@chromium.org" <svenva@chromium.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"platform-driver-x86@vger.kernel.org"
<platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH v5 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state
Date: Mon, 3 Apr 2023 13:07:43 -0500 [thread overview]
Message-ID: <3f7ec97b-5fa6-9c54-b4b2-58ebf4f88449@amd.com> (raw)
In-Reply-To: <59127d2ac2e60d59b5711517eb049eee334a3cdd.camel@intel.com>
On 4/3/2023 13:00, Box, David E wrote:
> On Fri, 2023-03-31 at 20:05 +0200, Rafael J. Wysocki wrote:
>> On Thu, Mar 30, 2023 at 9:45 PM Mario Limonciello
>> <mario.limonciello@amd.com> 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>
>>> ---
>>> v4->v5:
>>> * Reword commit message
>>> ---
>>> drivers/platform/x86/intel/pmc/core.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/intel/pmc/core.c
>>> b/drivers/platform/x86/intel/pmc/core.c
>>> index e2f171fac094..980af32dd48a 100644
>>> --- a/drivers/platform/x86/intel/pmc/core.c
>>> +++ b/drivers/platform/x86/intel/pmc/core.c
>>> @@ -1203,6 +1203,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_set_hw_sleep_time(s0ix_counter - pmcdev->s0ix_counter);
>>> +
>>
>> Maybe check if this is really accumulating? In case of a counter
>> overflow, for instance?
>
> Overflow is likely on some systems. The counter is only 32-bit and at our
> smallest granularity of 30.5us per tick it could overflow after a day and a half
> of s0ix time, though most of our systems have a higher granularity that puts
> them around 6 days.
>
> This brings up an issue that the attribute cannot be trusted if the system is
> suspended for longer than the maximum hardware counter time. Should be noted in
> the Documentation.
I think it would be rather confusing for userspace having to account for
this and it's better to abstract it in the kernel.
How can you discover the granularity a system can support?
How would you know overflow actually happened? Is there a bit somewhere
else that could tell you?
In terms of ABI how about when we know overflow occurred and userspace
reads the sysfs file we return -EOVERFLOW instead of a potentially bad
value?
next prev parent reply other threads:[~2023-04-03 18:07 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-30 19:44 [PATCH v5 0/4] Add vendor agnostic mechanism to report hardware sleep Mario Limonciello
2023-03-30 19:44 ` [PATCH v5 1/4] PM: Add a sysfs file to represent time spent in hardware sleep state Mario Limonciello
2023-03-31 18:01 ` Rafael J. Wysocki
2023-03-31 18:05 ` Limonciello, Mario
2023-03-31 18:07 ` Rafael J. Wysocki
2023-03-31 18:13 ` Limonciello, Mario
2023-03-31 18:25 ` Rafael J. Wysocki
2023-03-30 19:44 ` [PATCH v5 2/4] platform/x86/amd: pmc: Report duration of time in hw " Mario Limonciello
2023-03-30 19:44 ` [PATCH v5 3/4] platform/x86/intel/pmc: core: Always capture counters on suspend Mario Limonciello
2023-04-03 16:58 ` Box, David E
2023-03-30 19:44 ` [PATCH v5 4/4] platform/x86/intel/pmc: core: Report duration of time in HW sleep state Mario Limonciello
2023-03-31 3:02 ` kernel test robot
2023-03-31 18:05 ` Rafael J. Wysocki
2023-04-03 18:00 ` Box, David E
2023-04-03 18:07 ` Limonciello, Mario [this message]
2023-04-03 18:32 ` Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3f7ec97b-5fa6-9c54-b4b2-58ebf4f88449@amd.com \
--to=mario.limonciello@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=david.e.box@intel.com \
--cc=hdegoede@redhat.com \
--cc=irenic.rajneesh@gmail.com \
--cc=jstultz@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=rajatja@google.com \
--cc=rrangel@chromium.org \
--cc=svenva@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).