Re: [PATCH] ACPI: x86: Adjust Microsoft LPS0 _DSM handling sequence

["David E. Box" <david.e.box@linux.intel.com>]

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,