* [PATCH v2 0/4] Add some extra debugging mechanisms for s0i3 @ 2022-08-29 13:29 Mario Limonciello 2022-08-29 13:29 ` [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops Mario Limonciello 0 siblings, 1 reply; 6+ messages in thread From: Mario Limonciello @ 2022-08-29 13:29 UTC (permalink / raw) To: mario.limonciello, Len Brown, linux-acpi, platform-driver-x86, linux-pm Cc: rafael, hdegoede, linux-kernel Recently there have been reports of problems where the system consumes too much power after certain interrupts occur that would notify the kernel of some event but those events aren't marked for wakeup. These problems have been root caused to the timing of the kernel moving the cores into ACPI C3 relative to other events from the previous wakeup not being settled. Linux will more aggressively move the cores into C3 for s2idle than Windows does for Modern Standby. To aide with debugging this class of problems in the future add a new set of optional debugging infrastructure. Mario Limonciello (4): ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops platform/x86/amd: pmc: Add defines for STB events platform/x86/amd: pmc: Always write to the STB platform/x86/amd: pmc: Add an extra STB message for entering s2idle drivers/acpi/sleep.h | 1 + drivers/acpi/x86/s2idle.c | 14 ++++++++++++++ drivers/platform/x86/amd/pmc.c | 32 ++++++++++++++++++++------------ include/linux/acpi.h | 1 + include/linux/suspend.h | 1 + kernel/power/suspend.c | 3 +++ 6 files changed, 40 insertions(+), 12 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops 2022-08-29 13:29 [PATCH v2 0/4] Add some extra debugging mechanisms for s0i3 Mario Limonciello @ 2022-08-29 13:29 ` Mario Limonciello 2022-08-29 14:20 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Mario Limonciello @ 2022-08-29 13:29 UTC (permalink / raw) To: mario.limonciello, Rafael J. Wysocki, Len Brown, Pavel Machek Cc: hdegoede, linux-acpi, linux-kernel, linux-pm On some platforms it is found that Linux more aggressively enters s2idle than Windows enters Modern Standby and this uncovers some synchronization issues for the platform. To aid in debugging this class of problems in the future, add support for an extra optional callback intended for drivers to emit extra debugging. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- v1->v2: * Add a prototype for `acpi_s2idle_enter` drivers/acpi/sleep.h | 1 + drivers/acpi/x86/s2idle.c | 14 ++++++++++++++ include/linux/acpi.h | 1 + include/linux/suspend.h | 1 + kernel/power/suspend.c | 3 +++ 5 files changed, 20 insertions(+) diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h index 7fe41ee489d6..7856930a7da9 100644 --- a/drivers/acpi/sleep.h +++ b/drivers/acpi/sleep.h @@ -18,6 +18,7 @@ static inline acpi_status acpi_set_waking_vector(u32 wakeup_address) extern int acpi_s2idle_begin(void); extern int acpi_s2idle_prepare(void); extern int acpi_s2idle_prepare_late(void); +extern void acpi_s2idle_enter(void); extern bool acpi_s2idle_wake(void); extern void acpi_s2idle_restore_early(void); extern void acpi_s2idle_restore(void); diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c index f9ac12b778e6..c984093a3657 100644 --- a/drivers/acpi/x86/s2idle.c +++ b/drivers/acpi/x86/s2idle.c @@ -486,6 +486,19 @@ int acpi_s2idle_prepare_late(void) return 0; } +void acpi_s2idle_enter(void) +{ + struct acpi_s2idle_dev_ops *handler; + + if (!lps0_device_handle || sleep_no_lps0) + return; + + list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { + if (handler->enter) + handler->enter(); + } +} + void acpi_s2idle_restore_early(void) { struct acpi_s2idle_dev_ops *handler; @@ -527,6 +540,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = { .begin = acpi_s2idle_begin, .prepare = acpi_s2idle_prepare, .prepare_late = acpi_s2idle_prepare_late, + .enter = acpi_s2idle_enter, .wake = acpi_s2idle_wake, .restore_early = acpi_s2idle_restore_early, .restore = acpi_s2idle_restore, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 6f64b2f3dc54..9df14b5a875c 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -1076,6 +1076,7 @@ struct acpi_s2idle_dev_ops { struct list_head list_node; void (*prepare)(void); void (*restore)(void); + void (*enter)(void); }; int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg); void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg); diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 70f2921e2e70..5a3fdca0a628 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -191,6 +191,7 @@ struct platform_s2idle_ops { int (*begin)(void); int (*prepare)(void); int (*prepare_late)(void); + void (*enter)(void); bool (*wake)(void); void (*restore_early)(void); void (*restore)(void); diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index 827075944d28..0c08032d6b50 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -136,6 +136,9 @@ static void s2idle_loop(void) break; } + if (s2idle_ops && s2idle_ops->enter) + s2idle_ops->enter(); + s2idle_enter(); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops 2022-08-29 13:29 ` [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops Mario Limonciello @ 2022-08-29 14:20 ` Rafael J. Wysocki 2022-08-29 14:32 ` Limonciello, Mario 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2022-08-29 14:20 UTC (permalink / raw) To: Mario Limonciello Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Hans de Goede, ACPI Devel Maling List, Linux Kernel Mailing List, Linux PM On Mon, Aug 29, 2022 at 3:29 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > On some platforms it is found that Linux more aggressively enters s2idle > than Windows enters Modern Standby and this uncovers some synchronization > issues for the platform. To aid in debugging this class of problems in > the future, add support for an extra optional callback intended for > drivers to emit extra debugging. I'm not liking this. If you want debug, why not simply add it where it is needed? > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > v1->v2: > * Add a prototype for `acpi_s2idle_enter` > > drivers/acpi/sleep.h | 1 + > drivers/acpi/x86/s2idle.c | 14 ++++++++++++++ > include/linux/acpi.h | 1 + > include/linux/suspend.h | 1 + > kernel/power/suspend.c | 3 +++ > 5 files changed, 20 insertions(+) > > diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h > index 7fe41ee489d6..7856930a7da9 100644 > --- a/drivers/acpi/sleep.h > +++ b/drivers/acpi/sleep.h > @@ -18,6 +18,7 @@ static inline acpi_status acpi_set_waking_vector(u32 wakeup_address) > extern int acpi_s2idle_begin(void); > extern int acpi_s2idle_prepare(void); > extern int acpi_s2idle_prepare_late(void); > +extern void acpi_s2idle_enter(void); And this name is confusing, because it suggests that the role of the callback is to make the platform enter s2idle which isn't the case. > extern bool acpi_s2idle_wake(void); > extern void acpi_s2idle_restore_early(void); > extern void acpi_s2idle_restore(void); > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > index f9ac12b778e6..c984093a3657 100644 > --- a/drivers/acpi/x86/s2idle.c > +++ b/drivers/acpi/x86/s2idle.c > @@ -486,6 +486,19 @@ int acpi_s2idle_prepare_late(void) > return 0; > } > > +void acpi_s2idle_enter(void) > +{ > + struct acpi_s2idle_dev_ops *handler; > + > + if (!lps0_device_handle || sleep_no_lps0) > + return; > + > + list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { > + if (handler->enter) > + handler->enter(); > + } > +} > + > void acpi_s2idle_restore_early(void) > { > struct acpi_s2idle_dev_ops *handler; > @@ -527,6 +540,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = { > .begin = acpi_s2idle_begin, > .prepare = acpi_s2idle_prepare, > .prepare_late = acpi_s2idle_prepare_late, > + .enter = acpi_s2idle_enter, > .wake = acpi_s2idle_wake, > .restore_early = acpi_s2idle_restore_early, > .restore = acpi_s2idle_restore, > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 6f64b2f3dc54..9df14b5a875c 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -1076,6 +1076,7 @@ struct acpi_s2idle_dev_ops { > struct list_head list_node; > void (*prepare)(void); > void (*restore)(void); > + void (*enter)(void); > }; > int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg); > void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg); > diff --git a/include/linux/suspend.h b/include/linux/suspend.h > index 70f2921e2e70..5a3fdca0a628 100644 > --- a/include/linux/suspend.h > +++ b/include/linux/suspend.h > @@ -191,6 +191,7 @@ struct platform_s2idle_ops { > int (*begin)(void); > int (*prepare)(void); > int (*prepare_late)(void); > + void (*enter)(void); > bool (*wake)(void); > void (*restore_early)(void); > void (*restore)(void); > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > index 827075944d28..0c08032d6b50 100644 > --- a/kernel/power/suspend.c > +++ b/kernel/power/suspend.c > @@ -136,6 +136,9 @@ static void s2idle_loop(void) > break; > } > > + if (s2idle_ops && s2idle_ops->enter) > + s2idle_ops->enter(); > + > s2idle_enter(); > } > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops 2022-08-29 14:20 ` Rafael J. Wysocki @ 2022-08-29 14:32 ` Limonciello, Mario 2022-08-29 14:40 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Limonciello, Mario @ 2022-08-29 14:32 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Len Brown, Pavel Machek, Hans de Goede, ACPI Devel Maling List, Linux Kernel Mailing List, Linux PM On 8/29/2022 09:20, Rafael J. Wysocki wrote: > On Mon, Aug 29, 2022 at 3:29 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> On some platforms it is found that Linux more aggressively enters s2idle >> than Windows enters Modern Standby and this uncovers some synchronization >> issues for the platform. To aid in debugging this class of problems in >> the future, add support for an extra optional callback intended for >> drivers to emit extra debugging. > > I'm not liking this. > > If you want debug, why not simply add it where it is needed? Actually this is exactly where it's needed. The synchronization issue is with what non-x86 is doing while x86 transitions into ACPI C3. > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> v1->v2: >> * Add a prototype for `acpi_s2idle_enter` >> >> drivers/acpi/sleep.h | 1 + >> drivers/acpi/x86/s2idle.c | 14 ++++++++++++++ >> include/linux/acpi.h | 1 + >> include/linux/suspend.h | 1 + >> kernel/power/suspend.c | 3 +++ >> 5 files changed, 20 insertions(+) >> >> diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h >> index 7fe41ee489d6..7856930a7da9 100644 >> --- a/drivers/acpi/sleep.h >> +++ b/drivers/acpi/sleep.h >> @@ -18,6 +18,7 @@ static inline acpi_status acpi_set_waking_vector(u32 wakeup_address) >> extern int acpi_s2idle_begin(void); >> extern int acpi_s2idle_prepare(void); >> extern int acpi_s2idle_prepare_late(void); >> +extern void acpi_s2idle_enter(void); > > And this name is confusing, because it suggests that the role of the > callback is to make the platform enter s2idle which isn't the case. Can you propose another name? > >> extern bool acpi_s2idle_wake(void); >> extern void acpi_s2idle_restore_early(void); >> extern void acpi_s2idle_restore(void); >> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c >> index f9ac12b778e6..c984093a3657 100644 >> --- a/drivers/acpi/x86/s2idle.c >> +++ b/drivers/acpi/x86/s2idle.c >> @@ -486,6 +486,19 @@ int acpi_s2idle_prepare_late(void) >> return 0; >> } >> >> +void acpi_s2idle_enter(void) >> +{ >> + struct acpi_s2idle_dev_ops *handler; >> + >> + if (!lps0_device_handle || sleep_no_lps0) >> + return; >> + >> + list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { >> + if (handler->enter) >> + handler->enter(); >> + } >> +} >> + >> void acpi_s2idle_restore_early(void) >> { >> struct acpi_s2idle_dev_ops *handler; >> @@ -527,6 +540,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = { >> .begin = acpi_s2idle_begin, >> .prepare = acpi_s2idle_prepare, >> .prepare_late = acpi_s2idle_prepare_late, >> + .enter = acpi_s2idle_enter, >> .wake = acpi_s2idle_wake, >> .restore_early = acpi_s2idle_restore_early, >> .restore = acpi_s2idle_restore, >> diff --git a/include/linux/acpi.h b/include/linux/acpi.h >> index 6f64b2f3dc54..9df14b5a875c 100644 >> --- a/include/linux/acpi.h >> +++ b/include/linux/acpi.h >> @@ -1076,6 +1076,7 @@ struct acpi_s2idle_dev_ops { >> struct list_head list_node; >> void (*prepare)(void); >> void (*restore)(void); >> + void (*enter)(void); >> }; >> int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg); >> void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg); >> diff --git a/include/linux/suspend.h b/include/linux/suspend.h >> index 70f2921e2e70..5a3fdca0a628 100644 >> --- a/include/linux/suspend.h >> +++ b/include/linux/suspend.h >> @@ -191,6 +191,7 @@ struct platform_s2idle_ops { >> int (*begin)(void); >> int (*prepare)(void); >> int (*prepare_late)(void); >> + void (*enter)(void); >> bool (*wake)(void); >> void (*restore_early)(void); >> void (*restore)(void); >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c >> index 827075944d28..0c08032d6b50 100644 >> --- a/kernel/power/suspend.c >> +++ b/kernel/power/suspend.c >> @@ -136,6 +136,9 @@ static void s2idle_loop(void) >> break; >> } >> >> + if (s2idle_ops && s2idle_ops->enter) >> + s2idle_ops->enter(); >> + >> s2idle_enter(); >> } >> >> -- >> 2.34.1 >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops 2022-08-29 14:32 ` Limonciello, Mario @ 2022-08-29 14:40 ` Rafael J. Wysocki 2022-08-29 14:51 ` Limonciello, Mario 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2022-08-29 14:40 UTC (permalink / raw) To: Limonciello, Mario Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Hans de Goede, ACPI Devel Maling List, Linux Kernel Mailing List, Linux PM On Mon, Aug 29, 2022 at 4:32 PM Limonciello, Mario <mario.limonciello@amd.com> wrote: > > On 8/29/2022 09:20, Rafael J. Wysocki wrote: > > On Mon, Aug 29, 2022 at 3:29 PM Mario Limonciello > > <mario.limonciello@amd.com> wrote: > >> > >> On some platforms it is found that Linux more aggressively enters s2idle > >> than Windows enters Modern Standby and this uncovers some synchronization > >> issues for the platform. To aid in debugging this class of problems in > >> the future, add support for an extra optional callback intended for > >> drivers to emit extra debugging. > > > > I'm not liking this. > > > > If you want debug, why not simply add it where it is needed? > > Actually this is exactly where it's needed. The synchronization issue > is with what non-x86 is doing while x86 transitions into ACPI C3. I meant directly, without adding new driver callbacks. Adding callbacks for debug only seems a bit excessive. But I guess the problem is that you need to ask the PMC about something then, don't you? > > > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > >> --- > >> v1->v2: > >> * Add a prototype for `acpi_s2idle_enter` > >> > >> drivers/acpi/sleep.h | 1 + > >> drivers/acpi/x86/s2idle.c | 14 ++++++++++++++ > >> include/linux/acpi.h | 1 + > >> include/linux/suspend.h | 1 + > >> kernel/power/suspend.c | 3 +++ > >> 5 files changed, 20 insertions(+) > >> > >> diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h > >> index 7fe41ee489d6..7856930a7da9 100644 > >> --- a/drivers/acpi/sleep.h > >> +++ b/drivers/acpi/sleep.h > >> @@ -18,6 +18,7 @@ static inline acpi_status acpi_set_waking_vector(u32 wakeup_address) > >> extern int acpi_s2idle_begin(void); > >> extern int acpi_s2idle_prepare(void); > >> extern int acpi_s2idle_prepare_late(void); > >> +extern void acpi_s2idle_enter(void); > > > > And this name is confusing, because it suggests that the role of the > > callback is to make the platform enter s2idle which isn't the case. > > Can you propose another name? acpi_s2idle_check() or similar. > > > >> extern bool acpi_s2idle_wake(void); > >> extern void acpi_s2idle_restore_early(void); > >> extern void acpi_s2idle_restore(void); > >> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > >> index f9ac12b778e6..c984093a3657 100644 > >> --- a/drivers/acpi/x86/s2idle.c > >> +++ b/drivers/acpi/x86/s2idle.c > >> @@ -486,6 +486,19 @@ int acpi_s2idle_prepare_late(void) > >> return 0; > >> } > >> > >> +void acpi_s2idle_enter(void) > >> +{ > >> + struct acpi_s2idle_dev_ops *handler; > >> + > >> + if (!lps0_device_handle || sleep_no_lps0) > >> + return; > >> + > >> + list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { > >> + if (handler->enter) > >> + handler->enter(); > >> + } > >> +} > >> + > >> void acpi_s2idle_restore_early(void) > >> { > >> struct acpi_s2idle_dev_ops *handler; > >> @@ -527,6 +540,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = { > >> .begin = acpi_s2idle_begin, > >> .prepare = acpi_s2idle_prepare, > >> .prepare_late = acpi_s2idle_prepare_late, > >> + .enter = acpi_s2idle_enter, > >> .wake = acpi_s2idle_wake, > >> .restore_early = acpi_s2idle_restore_early, > >> .restore = acpi_s2idle_restore, > >> diff --git a/include/linux/acpi.h b/include/linux/acpi.h > >> index 6f64b2f3dc54..9df14b5a875c 100644 > >> --- a/include/linux/acpi.h > >> +++ b/include/linux/acpi.h > >> @@ -1076,6 +1076,7 @@ struct acpi_s2idle_dev_ops { > >> struct list_head list_node; > >> void (*prepare)(void); > >> void (*restore)(void); > >> + void (*enter)(void); > >> }; > >> int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg); > >> void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg); > >> diff --git a/include/linux/suspend.h b/include/linux/suspend.h > >> index 70f2921e2e70..5a3fdca0a628 100644 > >> --- a/include/linux/suspend.h > >> +++ b/include/linux/suspend.h > >> @@ -191,6 +191,7 @@ struct platform_s2idle_ops { > >> int (*begin)(void); > >> int (*prepare)(void); > >> int (*prepare_late)(void); > >> + void (*enter)(void); > >> bool (*wake)(void); > >> void (*restore_early)(void); > >> void (*restore)(void); > >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > >> index 827075944d28..0c08032d6b50 100644 > >> --- a/kernel/power/suspend.c > >> +++ b/kernel/power/suspend.c > >> @@ -136,6 +136,9 @@ static void s2idle_loop(void) > >> break; > >> } > >> > >> + if (s2idle_ops && s2idle_ops->enter) > >> + s2idle_ops->enter(); > >> + > >> s2idle_enter(); > >> } > >> > >> -- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops 2022-08-29 14:40 ` Rafael J. Wysocki @ 2022-08-29 14:51 ` Limonciello, Mario 0 siblings, 0 replies; 6+ messages in thread From: Limonciello, Mario @ 2022-08-29 14:51 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Len Brown, Pavel Machek, Hans de Goede, ACPI Devel Maling List, Linux Kernel Mailing List, Linux PM On 8/29/2022 09:40, Rafael J. Wysocki wrote: > On Mon, Aug 29, 2022 at 4:32 PM Limonciello, Mario > <mario.limonciello@amd.com> wrote: >> >> On 8/29/2022 09:20, Rafael J. Wysocki wrote: >>> On Mon, Aug 29, 2022 at 3:29 PM Mario Limonciello >>> <mario.limonciello@amd.com> wrote: >>>> >>>> On some platforms it is found that Linux more aggressively enters s2idle >>>> than Windows enters Modern Standby and this uncovers some synchronization >>>> issues for the platform. To aid in debugging this class of problems in >>>> the future, add support for an extra optional callback intended for >>>> drivers to emit extra debugging. >>> >>> I'm not liking this. >>> >>> If you want debug, why not simply add it where it is needed? >> >> Actually this is exactly where it's needed. The synchronization issue >> is with what non-x86 is doing while x86 transitions into ACPI C3. > > I meant directly, without adding new driver callbacks. Adding > callbacks for debug only seems a bit excessive. > > But I guess the problem is that you need to ask the PMC about > something then, don't you? Yeah, that's exactly it. The code changes there won't make sense generically. I suppose it's possible to make a global symbol and only call it in the AMD case, but this approach should be a lot cleaner. I did CC you in the other 3 patches in the series (in platform-x86) so you can see exactly how it's used. Patch 4/4 is the one that uses this. https://patchwork.kernel.org/project/platform-driver-x86/patch/20220829132908.5254-5-mario.limonciello@amd.com/ > >>> >>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>>> --- >>>> v1->v2: >>>> * Add a prototype for `acpi_s2idle_enter` >>>> >>>> drivers/acpi/sleep.h | 1 + >>>> drivers/acpi/x86/s2idle.c | 14 ++++++++++++++ >>>> include/linux/acpi.h | 1 + >>>> include/linux/suspend.h | 1 + >>>> kernel/power/suspend.c | 3 +++ >>>> 5 files changed, 20 insertions(+) >>>> >>>> diff --git a/drivers/acpi/sleep.h b/drivers/acpi/sleep.h >>>> index 7fe41ee489d6..7856930a7da9 100644 >>>> --- a/drivers/acpi/sleep.h >>>> +++ b/drivers/acpi/sleep.h >>>> @@ -18,6 +18,7 @@ static inline acpi_status acpi_set_waking_vector(u32 wakeup_address) >>>> extern int acpi_s2idle_begin(void); >>>> extern int acpi_s2idle_prepare(void); >>>> extern int acpi_s2idle_prepare_late(void); >>>> +extern void acpi_s2idle_enter(void); >>> >>> And this name is confusing, because it suggests that the role of the >>> callback is to make the platform enter s2idle which isn't the case. >> >> Can you propose another name? > > acpi_s2idle_check() or similar. OK, will plan to change over to this name for v3. > >>> >>>> extern bool acpi_s2idle_wake(void); >>>> extern void acpi_s2idle_restore_early(void); >>>> extern void acpi_s2idle_restore(void); >>>> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c >>>> index f9ac12b778e6..c984093a3657 100644 >>>> --- a/drivers/acpi/x86/s2idle.c >>>> +++ b/drivers/acpi/x86/s2idle.c >>>> @@ -486,6 +486,19 @@ int acpi_s2idle_prepare_late(void) >>>> return 0; >>>> } >>>> >>>> +void acpi_s2idle_enter(void) >>>> +{ >>>> + struct acpi_s2idle_dev_ops *handler; >>>> + >>>> + if (!lps0_device_handle || sleep_no_lps0) >>>> + return; >>>> + >>>> + list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { >>>> + if (handler->enter) >>>> + handler->enter(); >>>> + } >>>> +} >>>> + >>>> void acpi_s2idle_restore_early(void) >>>> { >>>> struct acpi_s2idle_dev_ops *handler; >>>> @@ -527,6 +540,7 @@ static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = { >>>> .begin = acpi_s2idle_begin, >>>> .prepare = acpi_s2idle_prepare, >>>> .prepare_late = acpi_s2idle_prepare_late, >>>> + .enter = acpi_s2idle_enter, >>>> .wake = acpi_s2idle_wake, >>>> .restore_early = acpi_s2idle_restore_early, >>>> .restore = acpi_s2idle_restore, >>>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h >>>> index 6f64b2f3dc54..9df14b5a875c 100644 >>>> --- a/include/linux/acpi.h >>>> +++ b/include/linux/acpi.h >>>> @@ -1076,6 +1076,7 @@ struct acpi_s2idle_dev_ops { >>>> struct list_head list_node; >>>> void (*prepare)(void); >>>> void (*restore)(void); >>>> + void (*enter)(void); >>>> }; >>>> int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg); >>>> void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg); >>>> diff --git a/include/linux/suspend.h b/include/linux/suspend.h >>>> index 70f2921e2e70..5a3fdca0a628 100644 >>>> --- a/include/linux/suspend.h >>>> +++ b/include/linux/suspend.h >>>> @@ -191,6 +191,7 @@ struct platform_s2idle_ops { >>>> int (*begin)(void); >>>> int (*prepare)(void); >>>> int (*prepare_late)(void); >>>> + void (*enter)(void); >>>> bool (*wake)(void); >>>> void (*restore_early)(void); >>>> void (*restore)(void); >>>> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c >>>> index 827075944d28..0c08032d6b50 100644 >>>> --- a/kernel/power/suspend.c >>>> +++ b/kernel/power/suspend.c >>>> @@ -136,6 +136,9 @@ static void s2idle_loop(void) >>>> break; >>>> } >>>> >>>> + if (s2idle_ops && s2idle_ops->enter) >>>> + s2idle_ops->enter(); >>>> + >>>> s2idle_enter(); >>>> } >>>> >>>> -- ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-29 14:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-29 13:29 [PATCH v2 0/4] Add some extra debugging mechanisms for s0i3 Mario Limonciello 2022-08-29 13:29 ` [PATCH v2 1/4] ACPI: s2idle: Add a new ->enter() callback for platform_s2idle_ops Mario Limonciello 2022-08-29 14:20 ` Rafael J. Wysocki 2022-08-29 14:32 ` Limonciello, Mario 2022-08-29 14:40 ` Rafael J. Wysocki 2022-08-29 14:51 ` Limonciello, Mario
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox