Re: [PATCH v3 3/7] acpi/x86: s2idle: Add module parameter to prefer Microsoft GUID

[Rafael J. Wysocki]

On Fri, Sep 16, 2022 at 8:26 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
>
> OEMs have made some mistakes in the past for the AMD GUID support
> and not populated the method properly.  To add an escape hatch for
> this problem introduce a module parameter that can force using
> the Microsoft GUID.
>
> This is intentionally introduced to both Intel and AMD codepaths
> to allow using the parameter as a debugging tactic on either.
>
> Reviewed-by: Philipp Zabel <philipp.zabel@xxxxxxxxx>
> Tested-by: Philipp Zabel <philipp.zabel@xxxxxxxxx> # GA402RJ
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> v2->v3:
>  * Add tags
> ---
>  drivers/acpi/x86/s2idle.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 2c5931d247a2..127215150b84 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -27,6 +27,10 @@ static bool sleep_no_lps0 __read_mostly;
>  module_param(sleep_no_lps0, bool, 0644);
>  MODULE_PARM_DESC(sleep_no_lps0, "Do not use the special LPS0 device interface");
>
> +static bool prefer_microsoft_guid __read_mostly;

I would call this prefer_microsoft_dsm_guid and make changes below analogously.

> +module_param(prefer_microsoft_guid, bool, 0644);
> +MODULE_PARM_DESC(prefer_microsoft_guid, "Prefer selecting Microsoft GUID for LPS0 device");

And here: "Prefer using Microsoft GUID in LPS0 device _DSM evaluation".

> +
>  static const struct acpi_device_id lps0_device_ids[] = {
>         {"PNP0D80", },
>         {"", },
> @@ -402,6 +406,9 @@ static int lps0_device_attach(struct acpi_device *adev,
>         if (lps0_device_handle)
>                 return 0;
>
> +       lps0_dsm_func_mask_microsoft = validate_dsm(adev->handle,
> +                                                   ACPI_LPS0_DSM_UUID_MICROSOFT, 0,
> +                                                   &lps0_dsm_guid_microsoft);
>         if (acpi_s2idle_vendor_amd()) {
>                 static const struct acpi_device_id *dev_id;
>                 const struct amd_lps0_hid_device_data *data;
> @@ -416,16 +423,12 @@ static int lps0_device_attach(struct acpi_device *adev,
>                 rev_id = data->rev_id;
>                 lps0_dsm_func_mask = validate_dsm(adev->handle,
>                                         ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid);
> -               lps0_dsm_func_mask_microsoft = validate_dsm(adev->handle,
> -                                       ACPI_LPS0_DSM_UUID_MICROSOFT, 0,
> -                                       &lps0_dsm_guid_microsoft);
>                 if (lps0_dsm_func_mask > 0x3 && data->check_off_by_one) {
>                         lps0_dsm_func_mask = (lps0_dsm_func_mask << 1) | 0x1;
>                         acpi_handle_debug(adev->handle, "_DSM UUID %s: Adjusted function mask: 0x%x\n",
>                                           ACPI_LPS0_DSM_UUID_AMD, lps0_dsm_func_mask);
>                 } else if (lps0_dsm_func_mask_microsoft > 0 && data->prefer_amd_guid &&
> -                               (!strcmp(hid, "AMDI0007") ||
> -                                !strcmp(hid, "AMDI0008"))) {
> +                               !prefer_microsoft_guid) {
>                         lps0_dsm_func_mask_microsoft = -EINVAL;
>                         acpi_handle_debug(adev->handle, "_DSM Using AMD method\n");
>                 }
> @@ -433,7 +436,8 @@ static int lps0_device_attach(struct acpi_device *adev,
>                 rev_id = 1;
>                 lps0_dsm_func_mask = validate_dsm(adev->handle,
>                                         ACPI_LPS0_DSM_UUID, rev_id, &lps0_dsm_guid);
> -               lps0_dsm_func_mask_microsoft = -EINVAL;
> +               if (!prefer_microsoft_guid)
> +                       lps0_dsm_func_mask_microsoft = -EINVAL;
>         }
>
>         if (lps0_dsm_func_mask < 0 && lps0_dsm_func_mask_microsoft < 0)
> --
> 2.34.1
>