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

[Annie Li <annie.li@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]


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 {

[-- Attachment #2: Type: text/html, Size: 3365 bytes --]