2023-11-06 16:14:54

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH] rtc: cmos: Use ACPI alarm for non-Intel x86 systems too

Intel systems > 2015 have been configured to use ACPI alarm instead
of HPET to avoid s2idle issues.

Having HPET programmed for wakeup causes problems on AMD systems with
s2idle as well.

One particular case is that the systemd "SuspendThenHibernate" feature
doesn't work properly on the Framework 13" AMD model. Switching to
using ACPI alarm fixes the issue.

Adjust the quirk to apply to AMD/Hygon systems from 2021 onwards.
This matches what has been tested and is specifically to avoid potential
risk to older systems.

Cc: [email protected] # 6.1+
Reported-by: [email protected]
Reported-by: [email protected]
Closes: https://github.com/systemd/systemd/issues/24279
Reported-by: Kelvie Wong <[email protected]>
Closes: https://community.frame.work/t/systemd-suspend-then-hibernate-wakes-up-after-5-minutes/39392
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/rtc/rtc-cmos.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 228fb2d11c70..696cfa7025de 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -818,18 +818,24 @@ static void rtc_wake_off(struct device *dev)
}

#ifdef CONFIG_X86
-/* Enable use_acpi_alarm mode for Intel platforms no earlier than 2015 */
static void use_acpi_alarm_quirks(void)
{
- if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+ switch (boot_cpu_data.x86_vendor) {
+ case X86_VENDOR_INTEL:
+ if (dmi_get_bios_year() < 2015)
+ return;
+ break;
+ case X86_VENDOR_AMD:
+ case X86_VENDOR_HYGON:
+ if (dmi_get_bios_year() < 2021)
+ return;
+ break;
+ default:
return;
-
+ }
if (!is_hpet_enabled())
return;

- if (dmi_get_bios_year() < 2015)
- return;
-
use_acpi_alarm = true;
}
#else
--
2.34.1


2023-11-06 16:17:29

by Raul Rangel

[permalink] [raw]
Subject: Re: [PATCH] rtc: cmos: Use ACPI alarm for non-Intel x86 systems too

On Mon, Nov 6, 2023 at 9:14 AM Mario Limonciello
<[email protected]> wrote:
>
> Intel systems > 2015 have been configured to use ACPI alarm instead
> of HPET to avoid s2idle issues.
>
> Having HPET programmed for wakeup causes problems on AMD systems with
> s2idle as well.
>
> One particular case is that the systemd "SuspendThenHibernate" feature
> doesn't work properly on the Framework 13" AMD model. Switching to
> using ACPI alarm fixes the issue.
>
> Adjust the quirk to apply to AMD/Hygon systems from 2021 onwards.
> This matches what has been tested and is specifically to avoid potential
> risk to older systems.
>
> Cc: [email protected] # 6.1+
> Reported-by: [email protected]
> Reported-by: [email protected]
> Closes: https://github.com/systemd/systemd/issues/24279
> Reported-by: Kelvie Wong <[email protected]>
> Closes: https://community.frame.work/t/systemd-suspend-then-hibernate-wakes-up-after-5-minutes/39392
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
> drivers/rtc/rtc-cmos.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index 228fb2d11c70..696cfa7025de 100644
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -818,18 +818,24 @@ static void rtc_wake_off(struct device *dev)
> }
>
> #ifdef CONFIG_X86
> -/* Enable use_acpi_alarm mode for Intel platforms no earlier than 2015 */
> static void use_acpi_alarm_quirks(void)
> {
> - if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> + switch (boot_cpu_data.x86_vendor) {
> + case X86_VENDOR_INTEL:
> + if (dmi_get_bios_year() < 2015)
> + return;
> + break;
> + case X86_VENDOR_AMD:
> + case X86_VENDOR_HYGON:
> + if (dmi_get_bios_year() < 2021)
> + return;
> + break;
> + default:
> return;
> -
> + }
> if (!is_hpet_enabled())
> return;
>
> - if (dmi_get_bios_year() < 2015)
> - return;
> -
> use_acpi_alarm = true;
> }
> #else
> --
> 2.34.1
>
Acked-by: Raul E Rangel <[email protected]>

2023-11-17 17:37:04

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH] rtc: cmos: Use ACPI alarm for non-Intel x86 systems too


On Mon, 06 Nov 2023 10:23:10 -0600, Mario Limonciello wrote:
> Intel systems > 2015 have been configured to use ACPI alarm instead
> of HPET to avoid s2idle issues.
>
> Having HPET programmed for wakeup causes problems on AMD systems with
> s2idle as well.
>
> One particular case is that the systemd "SuspendThenHibernate" feature
> doesn't work properly on the Framework 13" AMD model. Switching to
> using ACPI alarm fixes the issue.
>
> [...]

Applied, thanks!

[1/1] rtc: cmos: Use ACPI alarm for non-Intel x86 systems too
commit: 3d762e21d56370a43478b55e604b4a83dd85aafc

Best regards,

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2023-11-17 22:57:21

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH] rtc: cmos: Use ACPI alarm for non-Intel x86 systems too

On 14/11/2023 18:15:02-0600, Mario Limonciello wrote:
> On 11/14/2023 16:28, Alexandre Belloni wrote:
> > On 14/11/2023 10:06:36+0100, Pavel Machek wrote:
> > > On Mon 2023-11-13 23:38:19, Alexandre Belloni wrote:
> > > > On 13/11/2023 15:36:28-0600, Mario Limonciello wrote:
> > > > > Now that the merge window is over, can this be picked up?
> > > > >
> > > >
> > > > I'd be happy to invoice AMD so they get a quick response time.
> > >
> > > That is a really bad joke.
> >
> > Why would it be a joke?
> >
> > From what I get this is an issue since 2021, I don't get how this is so
> > urgent that I get a ping less than 24h after the end of the merge
> > window.
>
> It's possibly longer; but I don't have a large enough sample to say that
> it's safe that far back.

Would this help this one:
https://bugzilla.kernel.org/show_bug.cgi?id=68331

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2023-11-17 23:02:53

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH] rtc: cmos: Use ACPI alarm for non-Intel x86 systems too

On 11/17/2023 16:57, Alexandre Belloni wrote:
> On 14/11/2023 18:15:02-0600, Mario Limonciello wrote:
>> On 11/14/2023 16:28, Alexandre Belloni wrote:
>>> On 14/11/2023 10:06:36+0100, Pavel Machek wrote:
>>>> On Mon 2023-11-13 23:38:19, Alexandre Belloni wrote:
>>>>> On 13/11/2023 15:36:28-0600, Mario Limonciello wrote:
>>>>>> Now that the merge window is over, can this be picked up?
>>>>>>
>>>>>
>>>>> I'd be happy to invoice AMD so they get a quick response time.
>>>>
>>>> That is a really bad joke.
>>>
>>> Why would it be a joke?
>>>
>>> From what I get this is an issue since 2021, I don't get how this is so
>>> urgent that I get a ping less than 24h after the end of the merge
>>> window.
>>
>> It's possibly longer; but I don't have a large enough sample to say that
>> it's safe that far back.
>
> Would this help this one:
> https://bugzilla.kernel.org/show_bug.cgi?id=68331
>

The BIOS cutoff is set to 2021 in my patch, so unless they have an
updated BIOS for that system I think it's unlikely to do anything.

They can certainly try with the kernel command line though to see if it
could help. If it does we could add another case for it or adjust
boundaries.

2023-11-17 23:43:58

by Raul Rangel

[permalink] [raw]
Subject: Re: [PATCH] rtc: cmos: Use ACPI alarm for non-Intel x86 systems too

On Fri, Nov 17, 2023 at 3:57 PM Alexandre Belloni
<[email protected]> wrote:
>
> On 14/11/2023 18:15:02-0600, Mario Limonciello wrote:
> > On 11/14/2023 16:28, Alexandre Belloni wrote:
> > > On 14/11/2023 10:06:36+0100, Pavel Machek wrote:
> > > > On Mon 2023-11-13 23:38:19, Alexandre Belloni wrote:
> > > > > On 13/11/2023 15:36:28-0600, Mario Limonciello wrote:
> > > > > > Now that the merge window is over, can this be picked up?
> > > > > >
> > > > >
> > > > > I'd be happy to invoice AMD so they get a quick response time.
> > > >
> > > > That is a really bad joke.
> > >
> > > Why would it be a joke?
> > >
> > > From what I get this is an issue since 2021, I don't get how this is so
> > > urgent that I get a ping less than 24h after the end of the merge
> > > window.
> >
> > It's possibly longer; but I don't have a large enough sample to say that
> > it's safe that far back.
>
> Would this help this one:
> https://bugzilla.kernel.org/show_bug.cgi?id=68331


Honestly, the HPET_EMULATE_RTC code is what drove me to switch the AMD
chromebooks over to using the ACPI timer:
https://chromium-review.googlesource.com/c/chromiumos/overlays/board-overlays/+/2355073.

Ideally if the HPET was configured with proper IRQs by the firmware,
the kernel wouldn't enable the HPET legacy emulation bit. The HPET
spec defines that when the legacy replacement option bit is set, the
HPET takes control of the RTC (#8) and timer (#2) interrupts. Why it
takes over the RTC interrupt I've never understood. The HPET is not an
RTC, so it results in software having to emulate it which results in
extra complexity. If the kernel didn't set the HPET legacy emulation
bit when the HPET declared proper IRQs, then the RTC wake interrupt
would keep working as expected. I guess there's not much benefit to
fixing this anymore, now that this ACPI timer patch has landed. If a
platform were to release new firmware to define proper HPET IRQs, the
date would bump, and it would fall into the ACPI path.

>
>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com