diff mbox series

[RFC,V2,04/11] acpi: Support Control Method sleep button for x86

Message ID 20240927183906.1248-5-annie.li@oracle.com (mailing list archive)
State New, archived
Headers show
Series Support ACPI Control Method Sleep button | expand

Commit Message

Annie Li Sept. 27, 2024, 6:38 p.m. UTC
Adding Control Method Sleep button and its GPE event handler for
x86.

Signed-off-by: Annie Li <annie.li@oracle.com>
---
 hw/i386/acpi-build.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Igor Mammedov Oct. 7, 2024, 1:32 p.m. UTC | #1
On Fri, 27 Sep 2024 14:38:59 -0400
Annie Li <annie.li@oracle.com> wrote:

> Adding Control Method Sleep button and its GPE event handler for
> x86.
> 
> Signed-off-by: Annie Li <annie.li@oracle.com>
> ---
>  hw/i386/acpi-build.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5d4bd2b710..ee62333a03 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -40,6 +40,7 @@
>  #include "hw/acpi/acpi_aml_interface.h"
>  #include "hw/input/i8042.h"
>  #include "hw/acpi/memory_hotplug.h"
> +#include "hw/acpi/control_method_device.h"
>  #include "sysemu/tpm.h"
>  #include "hw/acpi/tpm.h"
>  #include "hw/acpi/vmgenid.h"
> @@ -1527,6 +1528,14 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>      }
>      aml_append(dsdt, scope);
>  
> +    sb_scope = aml_scope("_SB");
> +    acpi_dsdt_add_sleep_button(sb_scope);
> +    aml_append(dsdt, sb_scope);
> +
> +    scope =  aml_scope("\\_GPE");

> +    acpi_dsdt_add_sleep_gpe_event_handler(scope);
instead of having 1-off function, it would be better to inline impl,
here, like we do elsewhere with _Exx methods.

Also given that, series doesn't support wake,
you don't need to copy example from spec related to operation region,
as dedicated GPE event by itself tells us that sleep was requested.

so all you need to do is sending Notify to sleep button device.


> +    aml_append(dsdt, scope);
> +
>      if (pcmc->legacy_cpu_hotplug) {
>          build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
>      } else {
Annie Li Oct. 8, 2024, 3:33 p.m. UTC | #2
On 10/7/2024 9:32 AM, Igor Mammedov wrote:
> On Fri, 27 Sep 2024 14:38:59 -0400
> Annie Li<annie.li@oracle.com> wrote:
>
>> Adding Control Method Sleep button and its GPE event handler for
>> x86.
>>
>> Signed-off-by: Annie Li<annie.li@oracle.com>
>> ---
>>   hw/i386/acpi-build.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 5d4bd2b710..ee62333a03 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -40,6 +40,7 @@
>>   #include "hw/acpi/acpi_aml_interface.h"
>>   #include "hw/input/i8042.h"
>>   #include "hw/acpi/memory_hotplug.h"
>> +#include "hw/acpi/control_method_device.h"
>>   #include "sysemu/tpm.h"
>>   #include "hw/acpi/tpm.h"
>>   #include "hw/acpi/vmgenid.h"
>> @@ -1527,6 +1528,14 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>       }
>>       aml_append(dsdt, scope);
>>   
>> +    sb_scope = aml_scope("_SB");
>> +    acpi_dsdt_add_sleep_button(sb_scope);
>> +    aml_append(dsdt, sb_scope);
>> +
>> +    scope =  aml_scope("\\_GPE");
>> +    acpi_dsdt_add_sleep_gpe_event_handler(scope);
> instead of having 1-off function, it would be better to inline impl,
> here, like we do elsewhere with _Exx methods.
Was thinking that "acpi_dsdt_add_sleep_button" is shared by multiple 
platforms, and "acpi_dsdt_add_sleep_gpe_event_handler" is related this 
function. However, it totally makes sense to inline this function since 
it is only being used once for x86.
>
> Also given that, series doesn't support wake,
> you don't need to copy example from spec related to operation region,
> as dedicated GPE event by itself tells us that sleep was requested.
>
> so all you need to do is sending Notify to sleep button device.
Got it, thanks for the feedback.

Thanks
Annie
>
>> +    aml_append(dsdt, scope);
>> +
>>       if (pcmc->legacy_cpu_hotplug) {
>>           build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
>>       } else {
diff mbox series

Patch

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5d4bd2b710..ee62333a03 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -40,6 +40,7 @@ 
 #include "hw/acpi/acpi_aml_interface.h"
 #include "hw/input/i8042.h"
 #include "hw/acpi/memory_hotplug.h"
+#include "hw/acpi/control_method_device.h"
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
 #include "hw/acpi/vmgenid.h"
@@ -1527,6 +1528,14 @@  build_dsdt(GArray *table_data, BIOSLinker *linker,
     }
     aml_append(dsdt, scope);
 
+    sb_scope = aml_scope("_SB");
+    acpi_dsdt_add_sleep_button(sb_scope);
+    aml_append(dsdt, sb_scope);
+
+    scope =  aml_scope("\\_GPE");
+    acpi_dsdt_add_sleep_gpe_event_handler(scope);
+    aml_append(dsdt, scope);
+
     if (pcmc->legacy_cpu_hotplug) {
         build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
     } else {