| Message ID | 20230601233953.1332-1-mario.limonciello@amd.com |
|---|---|
| State | Accepted |
| Commit | f198478cfdc8105a1c8d8945918904f0498d19be |
| Headers | show |
| Series | ACPI: x86: Adjust Microsoft LPS0 _DSM handling sequence | expand |
On Thu, 2023-06-01 at 18:39 -0500, Mario Limonciello wrote: > In Windows the Microsoft _DSM doesn't call functions 3->5->7 for suspend > and 8->6->4 for resume like Linux currently does. > > Rather it calls 3->7->5 for suspend and 6->8->4 for resume. > Align this calling order for Linux as well. > > Link: > https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states I didn't catch the ordering in the link. Was there any issue that prompted this change? David > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/acpi/x86/s2idle.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > index e499c60c4579..7214197c15a0 100644 > --- a/drivers/acpi/x86/s2idle.c > +++ b/drivers/acpi/x86/s2idle.c > @@ -485,11 +485,11 @@ int acpi_s2idle_prepare_late(void) > ACPI_LPS0_ENTRY, > lps0_dsm_func_mask, lps0_dsm_guid); > if (lps0_dsm_func_mask_microsoft > 0) { > - acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, > - lps0_dsm_func_mask_microsoft, > lps0_dsm_guid_microsoft); > /* modern standby entry */ > acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY, > lps0_dsm_func_mask_microsoft, > lps0_dsm_guid_microsoft); > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, > + lps0_dsm_func_mask_microsoft, > lps0_dsm_guid_microsoft); > } > > list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { > @@ -524,11 +524,6 @@ void acpi_s2idle_restore_early(void) > if (handler->restore) > handler->restore(); > > - /* Modern standby exit */ > - if (lps0_dsm_func_mask_microsoft > 0) > - acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, > - lps0_dsm_func_mask_microsoft, > lps0_dsm_guid_microsoft); > - > /* LPS0 exit */ > if (lps0_dsm_func_mask > 0) > acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ? > @@ -539,6 +534,11 @@ void acpi_s2idle_restore_early(void) > acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT, > lps0_dsm_func_mask_microsoft, > lps0_dsm_guid_microsoft); > > + /* Modern standby exit */ > + if (lps0_dsm_func_mask_microsoft > 0) > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, > + lps0_dsm_func_mask_microsoft, > lps0_dsm_guid_microsoft); > + > /* Screen on */ > if (lps0_dsm_func_mask_microsoft > 0) > acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
On Thu, 2023-06-01 at 20:46 -0500, Limonciello, Mario wrote: > > On 6/1/2023 8:31 PM, David E. Box wrote: > > On Thu, 2023-06-01 at 18:39 -0500, Mario Limonciello wrote: > > > In Windows the Microsoft _DSM doesn't call functions 3->5->7 for suspend > > > and 8->6->4 for resume like Linux currently does. > > > > > > Rather it calls 3->7->5 for suspend and 6->8->4 for resume. > > > Align this calling order for Linux as well. > > > > > > Link: > > > https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states > > I didn't catch the ordering in the link. > > Yeah it's tough to interpret from the link, because the picture at the > bottom > is missing annotations. > > Basically if you look at the picture the blue part is the screen on/off. > > The green part is "modern standby" and then the little "humps" are LPS0 > enter/exit. > > > Was there any issue that prompted this > > change? > > > We were debugging an unrelated problem and noticed the difference > comparing the > > BIOS debugging log from Windows and Linux. > > If an OEM depends on this call order in that code used in LPS0 phase > requires > changes from MS phase I could hypothesize this fixes it. > > > > David > > BTW - is there interest in supporting the Microsoft _DSM GUID for Intel > side too? > > It's an incongruity today that we run both AMD GUID and Microsoft GUID > for AMD systems > but only run Intel GUID for Intel systems. There hasn't been a need yet. Rafael have you look at it? David > > > > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > > --- > > > drivers/acpi/x86/s2idle.c | 14 +++++++------- > > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > > > index e499c60c4579..7214197c15a0 100644 > > > --- a/drivers/acpi/x86/s2idle.c > > > +++ b/drivers/acpi/x86/s2idle.c > > > @@ -485,11 +485,11 @@ int acpi_s2idle_prepare_late(void) > > > ACPI_LPS0_ENTRY, > > > lps0_dsm_func_mask, > > > lps0_dsm_guid); > > > if (lps0_dsm_func_mask_microsoft > 0) { > > > - acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, > > > - lps0_dsm_func_mask_microsoft, > > > lps0_dsm_guid_microsoft); > > > /* modern standby entry */ > > > acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY, > > > lps0_dsm_func_mask_microsoft, > > > lps0_dsm_guid_microsoft); > > > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, > > > + lps0_dsm_func_mask_microsoft, > > > lps0_dsm_guid_microsoft); > > > } > > > > > > list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) > > > { > > > @@ -524,11 +524,6 @@ void acpi_s2idle_restore_early(void) > > > if (handler->restore) > > > handler->restore(); > > > > > > - /* Modern standby exit */ > > > - if (lps0_dsm_func_mask_microsoft > 0) > > > - acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, > > > - lps0_dsm_func_mask_microsoft, > > > lps0_dsm_guid_microsoft); > > > - > > > /* LPS0 exit */ > > > if (lps0_dsm_func_mask > 0) > > > acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ? > > > @@ -539,6 +534,11 @@ void acpi_s2idle_restore_early(void) > > > acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT, > > > lps0_dsm_func_mask_microsoft, > > > lps0_dsm_guid_microsoft); > > > > > > + /* Modern standby exit */ > > > + if (lps0_dsm_func_mask_microsoft > 0) > > > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, > > > + lps0_dsm_func_mask_microsoft, > > > lps0_dsm_guid_microsoft); > > > + > > > /* Screen on */ > > > if (lps0_dsm_func_mask_microsoft > 0) > > > acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
On 6/1/2023 10:06 PM, David E. Box wrote: > On Thu, 2023-06-01 at 20:46 -0500, Limonciello, Mario wrote: >> On 6/1/2023 8:31 PM, David E. Box wrote: >>> On Thu, 2023-06-01 at 18:39 -0500, Mario Limonciello wrote: >>>> In Windows the Microsoft _DSM doesn't call functions 3->5->7 for suspend >>>> and 8->6->4 for resume like Linux currently does. >>>> >>>> Rather it calls 3->7->5 for suspend and 6->8->4 for resume. >>>> Align this calling order for Linux as well. >>>> >>>> Link: >>>> https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states >>> I didn't catch the ordering in the link. >> Yeah it's tough to interpret from the link, because the picture at the >> bottom >> is missing annotations. >> >> Basically if you look at the picture the blue part is the screen on/off. >> >> The green part is "modern standby" and then the little "humps" are LPS0 >> enter/exit. >> >>> Was there any issue that prompted this >>> change? >> >> We were debugging an unrelated problem and noticed the difference >> comparing the >> >> BIOS debugging log from Windows and Linux. >> >> If an OEM depends on this call order in that code used in LPS0 phase >> requires >> changes from MS phase I could hypothesize this fixes it. >> >> >>> David >> BTW - is there interest in supporting the Microsoft _DSM GUID for Intel >> side too? >> >> It's an incongruity today that we run both AMD GUID and Microsoft GUID >> for AMD systems >> but only run Intel GUID for Intel systems. > There hasn't been a need yet. Rafael have you look at it? > > David AFAICT from Linux side it should be a one line patch to drop: lps0_dsm_func_mask_microsoft = -EINVAL; >>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>>> --- >>>> drivers/acpi/x86/s2idle.c | 14 +++++++------- >>>> 1 file changed, 7 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c >>>> index e499c60c4579..7214197c15a0 100644 >>>> --- a/drivers/acpi/x86/s2idle.c >>>> +++ b/drivers/acpi/x86/s2idle.c >>>> @@ -485,11 +485,11 @@ int acpi_s2idle_prepare_late(void) >>>> ACPI_LPS0_ENTRY, >>>> lps0_dsm_func_mask, >>>> lps0_dsm_guid); >>>> if (lps0_dsm_func_mask_microsoft > 0) { >>>> - acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, >>>> - lps0_dsm_func_mask_microsoft, >>>> lps0_dsm_guid_microsoft); >>>> /* modern standby entry */ >>>> acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY, >>>> lps0_dsm_func_mask_microsoft, >>>> lps0_dsm_guid_microsoft); >>>> + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, >>>> + lps0_dsm_func_mask_microsoft, >>>> lps0_dsm_guid_microsoft); >>>> } >>>> >>>> list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) >>>> { >>>> @@ -524,11 +524,6 @@ void acpi_s2idle_restore_early(void) >>>> if (handler->restore) >>>> handler->restore(); >>>> >>>> - /* Modern standby exit */ >>>> - if (lps0_dsm_func_mask_microsoft > 0) >>>> - acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, >>>> - lps0_dsm_func_mask_microsoft, >>>> lps0_dsm_guid_microsoft); >>>> - >>>> /* LPS0 exit */ >>>> if (lps0_dsm_func_mask > 0) >>>> acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ? >>>> @@ -539,6 +534,11 @@ void acpi_s2idle_restore_early(void) >>>> acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT, >>>> lps0_dsm_func_mask_microsoft, >>>> lps0_dsm_guid_microsoft); >>>> >>>> + /* Modern standby exit */ >>>> + if (lps0_dsm_func_mask_microsoft > 0) >>>> + acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, >>>> + lps0_dsm_func_mask_microsoft, >>>> lps0_dsm_guid_microsoft); >>>> + >>>> /* Screen on */ >>>> if (lps0_dsm_func_mask_microsoft > 0) >>>> acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
On Fri, Jun 2, 2023 at 1:40 AM Mario Limonciello <mario.limonciello@amd.com> wrote: > > In Windows the Microsoft _DSM doesn't call functions 3->5->7 for suspend > and 8->6->4 for resume like Linux currently does. > > Rather it calls 3->7->5 for suspend and 6->8->4 for resume. > Align this calling order for Linux as well. > > Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/acpi/x86/s2idle.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > index e499c60c4579..7214197c15a0 100644 > --- a/drivers/acpi/x86/s2idle.c > +++ b/drivers/acpi/x86/s2idle.c > @@ -485,11 +485,11 @@ int acpi_s2idle_prepare_late(void) > ACPI_LPS0_ENTRY, > lps0_dsm_func_mask, lps0_dsm_guid); > if (lps0_dsm_func_mask_microsoft > 0) { > - acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, > - lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); > /* modern standby entry */ > acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY, > lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, > + lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); > } > > list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { > @@ -524,11 +524,6 @@ void acpi_s2idle_restore_early(void) > if (handler->restore) > handler->restore(); > > - /* Modern standby exit */ > - if (lps0_dsm_func_mask_microsoft > 0) > - acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, > - lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); > - > /* LPS0 exit */ > if (lps0_dsm_func_mask > 0) > acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ? > @@ -539,6 +534,11 @@ void acpi_s2idle_restore_early(void) > acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT, > lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); > > + /* Modern standby exit */ > + if (lps0_dsm_func_mask_microsoft > 0) > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, > + lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); > + > /* Screen on */ > if (lps0_dsm_func_mask_microsoft > 0) > acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON, > -- Applied as 6.5 material, thanks!
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c index e499c60c4579..7214197c15a0 100644 --- a/drivers/acpi/x86/s2idle.c +++ b/drivers/acpi/x86/s2idle.c @@ -485,11 +485,11 @@ int acpi_s2idle_prepare_late(void) ACPI_LPS0_ENTRY, lps0_dsm_func_mask, lps0_dsm_guid); if (lps0_dsm_func_mask_microsoft > 0) { - acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, - lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); /* modern standby entry */ acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY, lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY, + lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); } list_for_each_entry(handler, &lps0_s2idle_devops_head, list_node) { @@ -524,11 +524,6 @@ void acpi_s2idle_restore_early(void) if (handler->restore) handler->restore(); - /* Modern standby exit */ - if (lps0_dsm_func_mask_microsoft > 0) - acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, - lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); - /* LPS0 exit */ if (lps0_dsm_func_mask > 0) acpi_sleep_run_lps0_dsm(acpi_s2idle_vendor_amd() ? @@ -539,6 +534,11 @@ void acpi_s2idle_restore_early(void) acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT, lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); + /* Modern standby exit */ + if (lps0_dsm_func_mask_microsoft > 0) + acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT, + lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft); + /* Screen on */ if (lps0_dsm_func_mask_microsoft > 0) acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
In Windows the Microsoft _DSM doesn't call functions 3->5->7 for suspend and 8->6->4 for resume like Linux currently does. Rather it calls 3->7->5 for suspend and 6->8->4 for resume. Align this calling order for Linux as well. Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-states Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/acpi/x86/s2idle.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)