Re: [PATCH 2/3] hw/loongarch/virt: Wire up S3 and S4 sleep

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 13 Jun 2024 18:30:16 +0100
Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:

> Wire up S3 and S4 sleep by setting relevant slp-typs bits for
> GED and generate _S3 and _S4 methods in acpi table.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/loongarch/acpi-build.c | 18 ++++++++++++++++++
>  hw/loongarch/virt.c       |  3 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> index af45ce526d6e..2cb1696b579b 100644
> --- a/hw/loongarch/acpi-build.c
> +++ b/hw/loongarch/acpi-build.c
> @@ -416,6 +416,24 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>      acpi_dsdt_add_tpm(dsdt, lvms);
>  #endif
>      /* System State Package */
> +    scope = aml_scope("\\");
> +    pkg = aml_package(4);
> +    aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S3));
> +    aml_append(pkg, aml_int(0)); /* ignored */
> +    aml_append(pkg, aml_int(0)); /* reserved */
> +    aml_append(pkg, aml_int(0)); /* reserved */
> +    aml_append(scope, aml_name_decl("_S3", pkg));
> +    aml_append(dsdt, scope);
> +
> +    scope = aml_scope("\\");
> +    pkg = aml_package(4);
> +    aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S4));
> +    aml_append(pkg, aml_int(0)); /* ignored */
> +    aml_append(pkg, aml_int(0)); /* reserved */
> +    aml_append(pkg, aml_int(0)); /* reserved */
> +    aml_append(scope, aml_name_decl("_S4", pkg));
> +    aml_append(dsdt, scope);

if it's the same in both patches, I'd suggest to move it
in helper function in hw/acpi/generic_event_device.c
and call that here.

>      scope = aml_scope("\\");
>      pkg = aml_package(4);
>      aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5));
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 66cef201abe0..a4b55f00a32b 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -615,6 +615,9 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic,
>      }
>      dev = qdev_new(TYPE_ACPI_GED);
>      qdev_prop_set_uint32(dev, "ged-event", event);
> +    qdev_prop_set_uint32(dev, "slp-typs", (1 << ACPI_GED_SLP_TYP_S3) |
> +                         (1 << ACPI_GED_SLP_TYP_S4) |
> +                         (1 << ACPI_GED_SLP_TYP_S5));

drop this, it should be default,
and backward compatibility for versioned machine types,
should be handled by compat machinery
 
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>  
>      /* ged event */
>