[PATCH 1/5] ACPI: PM: s2idle: Use correct revision id

[PATCH 1/5] ACPI: PM: s2idle: Use correct revision id

[Mario Limonciello]

From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

AMD spec mentions only revision 0. With this change,
device constraint list is populated properly.

Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---
 drivers/acpi/x86/s2idle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 2d7ddb8a8cb6..da27c1c45c9f 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -96,7 +96,7 @@ static void lpi_device_get_constraints_amd(void)
 	int i, j, k;
 
 	out_obj = acpi_evaluate_dsm_typed(lps0_device_handle, &lps0_dsm_guid,
-					  1, ACPI_LPS0_GET_DEVICE_CONSTRAINTS,
+					  rev_id, ACPI_LPS0_GET_DEVICE_CONSTRAINTS,
 					  NULL, ACPI_TYPE_PACKAGE);
 
 	if (!out_obj)
-- 
2.25.1

[PATCH 2/5] ACPI: PM: s2idle: Refactor common code

[Mario Limonciello]

From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

Refactor common code to prepare for upcoming changes.
* Remove unused struct.
* Print error before returning.
* Frees ACPI obj if _DSM type is not as expected.
* Treat lps0_dsm_func_mask as an integer rather than character
* Remove extra out_obj
* Move rev_id

Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---
 drivers/acpi/x86/s2idle.c | 67 ++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index da27c1c45c9f..c0cba025072f 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -49,7 +49,7 @@ static const struct acpi_device_id lps0_device_ids[] = {
 
 static acpi_handle lps0_device_handle;
 static guid_t lps0_dsm_guid;
-static char lps0_dsm_func_mask;
+static int lps0_dsm_func_mask;
 
 /* Device constraint entry structure */
 struct lpi_device_info {
@@ -70,15 +70,7 @@ struct lpi_constraints {
 	int min_dstate;
 };
 
-/* AMD */
-/* Device constraint entry structure */
-struct lpi_device_info_amd {
-	int revision;
-	int count;
-	union acpi_object *package;
-};
-
-/* Constraint package structure */
+/* AMD Constraint package structure */
 struct lpi_device_constraint_amd {
 	char *name;
 	int enabled;
@@ -99,12 +91,12 @@ static void lpi_device_get_constraints_amd(void)
 					  rev_id, ACPI_LPS0_GET_DEVICE_CONSTRAINTS,
 					  NULL, ACPI_TYPE_PACKAGE);
 
-	if (!out_obj)
-		return;
-
 	acpi_handle_debug(lps0_device_handle, "_DSM function 1 eval %s\n",
 			  out_obj ? "successful" : "failed");
 
+	if (!out_obj)
+		return;
+
 	for (i = 0; i < out_obj->package.count; i++) {
 		union acpi_object *package = &out_obj->package.elements[i];
 
@@ -336,11 +328,33 @@ static bool acpi_s2idle_vendor_amd(void)
 	return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
 }
 
+static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *dsm_guid)
+{
+	union acpi_object *obj;
+	int ret = -EINVAL;
+
+	guid_parse(uuid, dsm_guid);
+	obj = acpi_evaluate_dsm(handle, dsm_guid, rev, 0, NULL);
+
+	/* Check if the _DSM is present and as expected. */
+	if (!obj || obj->type != ACPI_TYPE_BUFFER || obj->buffer.length == 0 ||
+	    obj->buffer.length > sizeof(u32)) {
+		acpi_handle_debug(handle,
+				"_DSM UUID %s rev %d function 0 evaluation failed\n", uuid, rev);
+		goto out;
+	}
+
+	ret = *(int *)obj->buffer.pointer;
+	acpi_handle_debug(handle, "_DSM UUID %s rev %d function mask: 0x%x\n", uuid, rev, ret);
+
+out:
+	ACPI_FREE(obj);
+	return ret;
+}
+
 static int lps0_device_attach(struct acpi_device *adev,
 			      const struct acpi_device_id *not_used)
 {
-	union acpi_object *out_obj;
-
 	if (lps0_device_handle)
 		return 0;
 
@@ -348,28 +362,17 @@ static int lps0_device_attach(struct acpi_device *adev,
 		return 0;
 
 	if (acpi_s2idle_vendor_amd()) {
-		guid_parse(ACPI_LPS0_DSM_UUID_AMD, &lps0_dsm_guid);
-		out_obj = acpi_evaluate_dsm(adev->handle, &lps0_dsm_guid, 0, 0, NULL);
 		rev_id = 0;
+		lps0_dsm_func_mask = validate_dsm(adev->handle,
+					ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid);
 	} else {
-		guid_parse(ACPI_LPS0_DSM_UUID, &lps0_dsm_guid);
-		out_obj = acpi_evaluate_dsm(adev->handle, &lps0_dsm_guid, 1, 0, NULL);
 		rev_id = 1;
+		lps0_dsm_func_mask = validate_dsm(adev->handle,
+					ACPI_LPS0_DSM_UUID, rev_id, &lps0_dsm_guid);
 	}
 
-	/* Check if the _DSM is present and as expected. */
-	if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER) {
-		acpi_handle_debug(adev->handle,
-				  "_DSM function 0 evaluation failed\n");
-		return 0;
-	}
-
-	lps0_dsm_func_mask = *(char *)out_obj->buffer.pointer;
-
-	ACPI_FREE(out_obj);
-
-	acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
-			  lps0_dsm_func_mask);
+	if (lps0_dsm_func_mask < 0)
+		return 0;//function eval failed
 
 	lps0_device_handle = adev->handle;
 
-- 
2.25.1

Re: [PATCH 2/5] ACPI: PM: s2idle: Refactor common code

[Julian Sikorski]

W dniu 17.06.2021 o 18:42, Mario Limonciello pisze:
> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> 
> Refactor common code to prepare for upcoming changes.
> * Remove unused struct.
> * Print error before returning.
> * Frees ACPI obj if _DSM type is not as expected.
> * Treat lps0_dsm_func_mask as an integer rather than character
> * Remove extra out_obj
> * Move rev_id
> 
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> ---
>   drivers/acpi/x86/s2idle.c | 67 ++++++++++++++++++++-------------------
>   1 file changed, 35 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index da27c1c45c9f..c0cba025072f 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -49,7 +49,7 @@ static const struct acpi_device_id lps0_device_ids[] = {
>   
>   static acpi_handle lps0_device_handle;
>   static guid_t lps0_dsm_guid;
> -static char lps0_dsm_func_mask;
> +static int lps0_dsm_func_mask;
>   
>   /* Device constraint entry structure */
>   struct lpi_device_info {
> @@ -70,15 +70,7 @@ struct lpi_constraints {
>   	int min_dstate;
>   };
>   
> -/* AMD */
> -/* Device constraint entry structure */
> -struct lpi_device_info_amd {
> -	int revision;
> -	int count;
> -	union acpi_object *package;
> -};
> -
> -/* Constraint package structure */
> +/* AMD Constraint package structure */
>   struct lpi_device_constraint_amd {
>   	char *name;
>   	int enabled;
> @@ -99,12 +91,12 @@ static void lpi_device_get_constraints_amd(void)
>   					  rev_id, ACPI_LPS0_GET_DEVICE_CONSTRAINTS,
>   					  NULL, ACPI_TYPE_PACKAGE);
>   
> -	if (!out_obj)
> -		return;
> -
>   	acpi_handle_debug(lps0_device_handle, "_DSM function 1 eval %s\n",
>   			  out_obj ? "successful" : "failed");
>   
> +	if (!out_obj)
> +		return;
> +
>   	for (i = 0; i < out_obj->package.count; i++) {
>   		union acpi_object *package = &out_obj->package.elements[i];
>   
> @@ -336,11 +328,33 @@ static bool acpi_s2idle_vendor_amd(void)
>   	return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
>   }
>   
> +static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *dsm_guid)
> +{
> +	union acpi_object *obj;
> +	int ret = -EINVAL;
> +
> +	guid_parse(uuid, dsm_guid);
> +	obj = acpi_evaluate_dsm(handle, dsm_guid, rev, 0, NULL);
> +
> +	/* Check if the _DSM is present and as expected. */
> +	if (!obj || obj->type != ACPI_TYPE_BUFFER || obj->buffer.length == 0 ||
> +	    obj->buffer.length > sizeof(u32)) {
> +		acpi_handle_debug(handle,
> +				"_DSM UUID %s rev %d function 0 evaluation failed\n", uuid, rev);
> +		goto out;
> +	}
> +
> +	ret = *(int *)obj->buffer.pointer;
> +	acpi_handle_debug(handle, "_DSM UUID %s rev %d function mask: 0x%x\n", uuid, rev, ret);
> +
> +out:
> +	ACPI_FREE(obj);
> +	return ret;
> +}
> +
>   static int lps0_device_attach(struct acpi_device *adev,
>   			      const struct acpi_device_id *not_used)
>   {
> -	union acpi_object *out_obj;
> -
>   	if (lps0_device_handle)
>   		return 0;
>   
> @@ -348,28 +362,17 @@ static int lps0_device_attach(struct acpi_device *adev,
>   		return 0;
>   
>   	if (acpi_s2idle_vendor_amd()) {
> -		guid_parse(ACPI_LPS0_DSM_UUID_AMD, &lps0_dsm_guid);
> -		out_obj = acpi_evaluate_dsm(adev->handle, &lps0_dsm_guid, 0, 0, NULL);
>   		rev_id = 0;
> +		lps0_dsm_func_mask = validate_dsm(adev->handle,
> +					ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid);
>   	} else {
> -		guid_parse(ACPI_LPS0_DSM_UUID, &lps0_dsm_guid);
> -		out_obj = acpi_evaluate_dsm(adev->handle, &lps0_dsm_guid, 1, 0, NULL);
>   		rev_id = 1;
> +		lps0_dsm_func_mask = validate_dsm(adev->handle,
> +					ACPI_LPS0_DSM_UUID, rev_id, &lps0_dsm_guid);
>   	}
>   
> -	/* Check if the _DSM is present and as expected. */
> -	if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER) {
> -		acpi_handle_debug(adev->handle,
> -				  "_DSM function 0 evaluation failed\n");
> -		return 0;
> -	}
> -
> -	lps0_dsm_func_mask = *(char *)out_obj->buffer.pointer;
> -
> -	ACPI_FREE(out_obj);
> -
> -	acpi_handle_debug(adev->handle, "_DSM function mask: 0x%x\n",
> -			  lps0_dsm_func_mask);
> +	if (lps0_dsm_func_mask < 0)
> +		return 0;//function eval failed
>   
>   	lps0_device_handle = adev->handle;
>   
> 
Tested-by: Julian Sikorski <belegdol@gmail.com>

[PATCH 3/5] ACPI: PM: s2idle: Add support for multiple func mask

[Mario Limonciello]

From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

Required for follow-up patch adding new UUID
needing new function mask.

Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---
 drivers/acpi/x86/s2idle.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index c0cba025072f..0d19669ac7ad 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -309,14 +309,15 @@ static void lpi_check_constraints(void)
 	}
 }
 
-static void acpi_sleep_run_lps0_dsm(unsigned int func)
+static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, guid_t dsm_guid)
 {
 	union acpi_object *out_obj;
 
-	if (!(lps0_dsm_func_mask & (1 << func)))
+	if (!(func_mask & (1 << func)))
 		return;
 
-	out_obj = acpi_evaluate_dsm(lps0_device_handle, &lps0_dsm_guid, rev_id, func, NULL);
+	out_obj = acpi_evaluate_dsm(lps0_device_handle, &dsm_guid,
+					rev_id, func, NULL);
 	ACPI_FREE(out_obj);
 
 	acpi_handle_debug(lps0_device_handle, "_DSM function %u evaluation %s\n",
@@ -412,11 +413,15 @@ int acpi_s2idle_prepare_late(void)
 		lpi_check_constraints();
 
 	if (acpi_s2idle_vendor_amd()) {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
+				lps0_dsm_func_mask, lps0_dsm_guid);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
+				lps0_dsm_func_mask, lps0_dsm_guid);
 	} else {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
+				lps0_dsm_func_mask, lps0_dsm_guid);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
+				lps0_dsm_func_mask, lps0_dsm_guid);
 	}
 
 	return 0;
@@ -428,11 +433,15 @@ void acpi_s2idle_restore_early(void)
 		return;
 
 	if (acpi_s2idle_vendor_amd()) {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
+				lps0_dsm_func_mask, lps0_dsm_guid);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
+				lps0_dsm_func_mask, lps0_dsm_guid);
 	} else {
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
-		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
+				lps0_dsm_func_mask, lps0_dsm_guid);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
+				lps0_dsm_func_mask, lps0_dsm_guid);
 	}
 }
 
-- 
2.25.1

Re: [PATCH 3/5] ACPI: PM: s2idle: Add support for multiple func mask

[Julian Sikorski]

W dniu 17.06.2021 o 18:42, Mario Limonciello pisze:
> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> 
> Required for follow-up patch adding new UUID
> needing new function mask.
> 
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> ---
>   drivers/acpi/x86/s2idle.c | 31 ++++++++++++++++++++-----------
>   1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index c0cba025072f..0d19669ac7ad 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -309,14 +309,15 @@ static void lpi_check_constraints(void)
>   	}
>   }
>   
> -static void acpi_sleep_run_lps0_dsm(unsigned int func)
> +static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, guid_t dsm_guid)
>   {
>   	union acpi_object *out_obj;
>   
> -	if (!(lps0_dsm_func_mask & (1 << func)))
> +	if (!(func_mask & (1 << func)))
>   		return;
>   
> -	out_obj = acpi_evaluate_dsm(lps0_device_handle, &lps0_dsm_guid, rev_id, func, NULL);
> +	out_obj = acpi_evaluate_dsm(lps0_device_handle, &dsm_guid,
> +					rev_id, func, NULL);
>   	ACPI_FREE(out_obj);
>   
>   	acpi_handle_debug(lps0_device_handle, "_DSM function %u evaluation %s\n",
> @@ -412,11 +413,15 @@ int acpi_s2idle_prepare_late(void)
>   		lpi_check_constraints();
>   
>   	if (acpi_s2idle_vendor_amd()) {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
>   	} else {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
>   	}
>   
>   	return 0;
> @@ -428,11 +433,15 @@ void acpi_s2idle_restore_early(void)
>   		return;
>   
>   	if (acpi_s2idle_vendor_amd()) {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
>   	} else {
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
> -		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
> +				lps0_dsm_func_mask, lps0_dsm_guid);
>   	}
>   }
>   
> 
Tested-by: Julian Sikorski <belegdol@gmail.com>

Re: [PATCH 3/5] ACPI: PM: s2idle: Add support for multiple func mask

[Rafael J. Wysocki]

On Thu, Jun 17, 2021 at 6:42 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
>
> Required for follow-up patch adding new UUID
> needing new function mask.
>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

Your s-o-b is missing.

> ---
>  drivers/acpi/x86/s2idle.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index c0cba025072f..0d19669ac7ad 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -309,14 +309,15 @@ static void lpi_check_constraints(void)
>         }
>  }
>
> -static void acpi_sleep_run_lps0_dsm(unsigned int func)
> +static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, guid_t dsm_guid)
>  {
>         union acpi_object *out_obj;
>
> -       if (!(lps0_dsm_func_mask & (1 << func)))
> +       if (!(func_mask & (1 << func)))
>                 return;
>
> -       out_obj = acpi_evaluate_dsm(lps0_device_handle, &lps0_dsm_guid, rev_id, func, NULL);
> +       out_obj = acpi_evaluate_dsm(lps0_device_handle, &dsm_guid,
> +                                       rev_id, func, NULL);
>         ACPI_FREE(out_obj);
>
>         acpi_handle_debug(lps0_device_handle, "_DSM function %u evaluation %s\n",
> @@ -412,11 +413,15 @@ int acpi_s2idle_prepare_late(void)
>                 lpi_check_constraints();
>
>         if (acpi_s2idle_vendor_amd()) {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>         } else {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>         }
>
>         return 0;
> @@ -428,11 +433,15 @@ void acpi_s2idle_restore_early(void)
>                 return;
>
>         if (acpi_s2idle_vendor_amd()) {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>         } else {
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>         }
>  }
>
> --
> 2.25.1
>

Re: [PATCH 3/5] ACPI: PM: s2idle: Add support for multiple func mask

[Limonciello, Mario]

On 6/18/2021 11:30, Rafael J. Wysocki wrote:
> On Thu, Jun 17, 2021 at 6:42 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
>>
>> Required for follow-up patch adding new UUID
>> needing new function mask.
>>
>> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> 
> Your s-o-b is missing.

My apologies.  If I need to spin the series I'll explicitly add it with 
all the other tags that have come through, otherwise:

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

> 
>> ---
>>   drivers/acpi/x86/s2idle.c | 31 ++++++++++++++++++++-----------
>>   1 file changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
>> index c0cba025072f..0d19669ac7ad 100644
>> --- a/drivers/acpi/x86/s2idle.c
>> +++ b/drivers/acpi/x86/s2idle.c
>> @@ -309,14 +309,15 @@ static void lpi_check_constraints(void)
>>          }
>>   }
>>
>> -static void acpi_sleep_run_lps0_dsm(unsigned int func)
>> +static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, guid_t dsm_guid)
>>   {
>>          union acpi_object *out_obj;
>>
>> -       if (!(lps0_dsm_func_mask & (1 << func)))
>> +       if (!(func_mask & (1 << func)))
>>                  return;
>>
>> -       out_obj = acpi_evaluate_dsm(lps0_device_handle, &lps0_dsm_guid, rev_id, func, NULL);
>> +       out_obj = acpi_evaluate_dsm(lps0_device_handle, &dsm_guid,
>> +                                       rev_id, func, NULL);
>>          ACPI_FREE(out_obj);
>>
>>          acpi_handle_debug(lps0_device_handle, "_DSM function %u evaluation %s\n",
>> @@ -412,11 +413,15 @@ int acpi_s2idle_prepare_late(void)
>>                  lpi_check_constraints();
>>
>>          if (acpi_s2idle_vendor_amd()) {
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD);
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>>          } else {
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>>          }
>>
>>          return 0;
>> @@ -428,11 +433,15 @@ void acpi_s2idle_restore_early(void)
>>                  return;
>>
>>          if (acpi_s2idle_vendor_amd()) {
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD);
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>>          } else {
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
>> -               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>> +               acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
>> +                               lps0_dsm_func_mask, lps0_dsm_guid);
>>          }
>>   }
>>
>> --
>> 2.25.1
>>

Re: [PATCH 3/5] ACPI: PM: s2idle: Add support for multiple func mask

[Rafael J. Wysocki]

On Fri, Jun 18, 2021 at 6:34 PM Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
> On 6/18/2021 11:30, Rafael J. Wysocki wrote:
> > On Thu, Jun 17, 2021 at 6:42 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> >>
> >> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> >>
> >> Required for follow-up patch adding new UUID
> >> needing new function mask.
> >>
> >> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> >
> > Your s-o-b is missing.
>
> My apologies.  If I need to spin the series I'll explicitly add it with
> all the other tags that have come through, otherwise:
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

OK

Applied as 5.14 material along with the rest of the series, thanks!

[PATCH 4/5] ACPI: PM: s2idle: Add support for new Microsoft UUID

[Mario Limonciello]

From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

This adds supports for _DSM notifications to the Microsoft UUID
described by Microsoft documentation for s2idle.

Link: https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-firmware-notifications
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---
 drivers/acpi/x86/s2idle.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 0d19669ac7ad..3f2a90648ec9 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -32,6 +32,9 @@ static const struct acpi_device_id lps0_device_ids[] = {
 	{"", },
 };
 
+/* Microsoft platform agnostic UUID */
+#define ACPI_LPS0_DSM_UUID_MICROSOFT      "11e00d56-ce64-47ce-837b-1f898f9aa461"
+
 #define ACPI_LPS0_DSM_UUID	"c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"
 
 #define ACPI_LPS0_GET_DEVICE_CONSTRAINTS	1
@@ -39,6 +42,8 @@ static const struct acpi_device_id lps0_device_ids[] = {
 #define ACPI_LPS0_SCREEN_ON	4
 #define ACPI_LPS0_ENTRY		5
 #define ACPI_LPS0_EXIT		6
+#define ACPI_LPS0_MS_ENTRY      7
+#define ACPI_LPS0_MS_EXIT       8
 
 /* AMD */
 #define ACPI_LPS0_DSM_UUID_AMD      "e3f32452-febc-43ce-9039-932122d37721"
@@ -51,6 +56,9 @@ static acpi_handle lps0_device_handle;
 static guid_t lps0_dsm_guid;
 static int lps0_dsm_func_mask;
 
+static guid_t lps0_dsm_guid_microsoft;
+static int lps0_dsm_func_mask_microsoft;
+
 /* Device constraint entry structure */
 struct lpi_device_info {
 	char *name;
@@ -366,14 +374,18 @@ static int lps0_device_attach(struct acpi_device *adev,
 		rev_id = 0;
 		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, rev_id,
+					&lps0_dsm_guid_microsoft);
 	} else {
 		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 (lps0_dsm_func_mask < 0)
-		return 0;//function eval failed
+	if (lps0_dsm_func_mask < 0 && lps0_dsm_func_mask_microsoft < 0)
+		return 0; //function evaluation failed
 
 	lps0_device_handle = adev->handle;
 
@@ -412,7 +424,14 @@ int acpi_s2idle_prepare_late(void)
 	if (pm_debug_messages_on)
 		lpi_check_constraints();
 
-	if (acpi_s2idle_vendor_amd()) {
+	if (lps0_dsm_func_mask_microsoft > 0) {
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
+				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
+				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);
+	} else if (acpi_s2idle_vendor_amd()) {
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
 				lps0_dsm_func_mask, lps0_dsm_guid);
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
@@ -432,7 +451,14 @@ void acpi_s2idle_restore_early(void)
 	if (!lps0_device_handle || sleep_no_lps0)
 		return;
 
-	if (acpi_s2idle_vendor_amd()) {
+	if (lps0_dsm_func_mask_microsoft > 0) {
+		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
+				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+		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_SCREEN_ON,
+				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
+	} else if (acpi_s2idle_vendor_amd()) {
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
 				lps0_dsm_func_mask, lps0_dsm_guid);
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
-- 
2.25.1

Re: [PATCH 4/5] ACPI: PM: s2idle: Add support for new Microsoft UUID

[Julian Sikorski]

W dniu 17.06.2021 o 18:42, Mario Limonciello pisze:
> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> 
> This adds supports for _DSM notifications to the Microsoft UUID
> described by Microsoft documentation for s2idle.
> 
> Link: https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-firmware-notifications
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> ---
>   drivers/acpi/x86/s2idle.c | 34 ++++++++++++++++++++++++++++++----
>   1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 0d19669ac7ad..3f2a90648ec9 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -32,6 +32,9 @@ static const struct acpi_device_id lps0_device_ids[] = {
>   	{"", },
>   };
>   
> +/* Microsoft platform agnostic UUID */
> +#define ACPI_LPS0_DSM_UUID_MICROSOFT      "11e00d56-ce64-47ce-837b-1f898f9aa461"
> +
>   #define ACPI_LPS0_DSM_UUID	"c4eb40a0-6cd2-11e2-bcfd-0800200c9a66"
>   
>   #define ACPI_LPS0_GET_DEVICE_CONSTRAINTS	1
> @@ -39,6 +42,8 @@ static const struct acpi_device_id lps0_device_ids[] = {
>   #define ACPI_LPS0_SCREEN_ON	4
>   #define ACPI_LPS0_ENTRY		5
>   #define ACPI_LPS0_EXIT		6
> +#define ACPI_LPS0_MS_ENTRY      7
> +#define ACPI_LPS0_MS_EXIT       8
>   
>   /* AMD */
>   #define ACPI_LPS0_DSM_UUID_AMD      "e3f32452-febc-43ce-9039-932122d37721"
> @@ -51,6 +56,9 @@ static acpi_handle lps0_device_handle;
>   static guid_t lps0_dsm_guid;
>   static int lps0_dsm_func_mask;
>   
> +static guid_t lps0_dsm_guid_microsoft;
> +static int lps0_dsm_func_mask_microsoft;
> +
>   /* Device constraint entry structure */
>   struct lpi_device_info {
>   	char *name;
> @@ -366,14 +374,18 @@ static int lps0_device_attach(struct acpi_device *adev,
>   		rev_id = 0;
>   		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, rev_id,
> +					&lps0_dsm_guid_microsoft);
>   	} else {
>   		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 (lps0_dsm_func_mask < 0)
> -		return 0;//function eval failed
> +	if (lps0_dsm_func_mask < 0 && lps0_dsm_func_mask_microsoft < 0)
> +		return 0; //function evaluation failed
>   
>   	lps0_device_handle = adev->handle;
>   
> @@ -412,7 +424,14 @@ int acpi_s2idle_prepare_late(void)
>   	if (pm_debug_messages_on)
>   		lpi_check_constraints();
>   
> -	if (acpi_s2idle_vendor_amd()) {
> +	if (lps0_dsm_func_mask_microsoft > 0) {
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF,
> +				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
> +				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);
> +	} else if (acpi_s2idle_vendor_amd()) {
>   		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
>   				lps0_dsm_func_mask, lps0_dsm_guid);
>   		acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
> @@ -432,7 +451,14 @@ void acpi_s2idle_restore_early(void)
>   	if (!lps0_device_handle || sleep_no_lps0)
>   		return;
>   
> -	if (acpi_s2idle_vendor_amd()) {
> +	if (lps0_dsm_func_mask_microsoft > 0) {
> +		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT,
> +				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +		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_SCREEN_ON,
> +				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
> +	} else if (acpi_s2idle_vendor_amd()) {
>   		acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
>   				lps0_dsm_func_mask, lps0_dsm_guid);
>   		acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,
> 
Tested-by: Julian Sikorski <belegdol@gmail.com>

[PATCH 5/5] ACPI: PM: Adjust behavior for field problems on AMD systems

[Mario Limonciello]

Some AMD Systems with uPEP _HID AMD004/AMDI005 have an off by one bug
in their function mask return.  This means that they will call entrance
but not exit for matching functions.

Other AMD systems with this HID should use the Microsoft generic UUID.

AMD systems with uPEP HID AMDI006 should be using the Microsoft method.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/x86/s2idle.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 3f2a90648ec9..816bf2c34b7a 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -371,12 +371,27 @@ static int lps0_device_attach(struct acpi_device *adev,
 		return 0;
 
 	if (acpi_s2idle_vendor_amd()) {
+		/* AMD0004, AMDI0005:
+		 * - Should use rev_id 0x0
+		 * - function mask > 0x3: Should use AMD method, but has off by one bug
+		 * - function mask = 0x3: Should use Microsoft method
+		 * AMDI0006:
+		 * - should use rev_id 0x0
+		 * - function mask = 0x3: Should use Microsoft method
+		 */
+		const char *hid = acpi_device_hid(adev);
 		rev_id = 0;
 		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, rev_id,
 					&lps0_dsm_guid_microsoft);
+		if (lps0_dsm_func_mask > 0x3 && (!strcmp(hid, "AMD0004") ||
+						 !strcmp(hid, "AMDI0005"))) {
+			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 {
 		rev_id = 1;
 		lps0_dsm_func_mask = validate_dsm(adev->handle,
-- 
2.25.1

Re: [PATCH 5/5] ACPI: PM: Adjust behavior for field problems on AMD systems

[Julian Sikorski]

W dniu 17.06.2021 o 18:42, Mario Limonciello pisze:
> Some AMD Systems with uPEP _HID AMD004/AMDI005 have an off by one bug
> in their function mask return.  This means that they will call entrance
> but not exit for matching functions.
> 
> Other AMD systems with this HID should use the Microsoft generic UUID.
> 
> AMD systems with uPEP HID AMDI006 should be using the Microsoft method.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/acpi/x86/s2idle.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 3f2a90648ec9..816bf2c34b7a 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -371,12 +371,27 @@ static int lps0_device_attach(struct acpi_device *adev,
>   		return 0;
>   
>   	if (acpi_s2idle_vendor_amd()) {
> +		/* AMD0004, AMDI0005:
> +		 * - Should use rev_id 0x0
> +		 * - function mask > 0x3: Should use AMD method, but has off by one bug
> +		 * - function mask = 0x3: Should use Microsoft method
> +		 * AMDI0006:
> +		 * - should use rev_id 0x0
> +		 * - function mask = 0x3: Should use Microsoft method
> +		 */
> +		const char *hid = acpi_device_hid(adev);
>   		rev_id = 0;
>   		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, rev_id,
>   					&lps0_dsm_guid_microsoft);
> +		if (lps0_dsm_func_mask > 0x3 && (!strcmp(hid, "AMD0004") ||
> +						 !strcmp(hid, "AMDI0005"))) {
> +			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 {
>   		rev_id = 1;
>   		lps0_dsm_func_mask = validate_dsm(adev->handle,
> 
Tested-by: Julian Sikorski <belegdol@gmail.com>

Re: [PATCH 5/5] ACPI: PM: Adjust behavior for field problems on AMD systems

[Kai-Heng Feng]

On 6/18/21 12:42 AM, Mario Limonciello wrote:
> Some AMD Systems with uPEP _HID AMD004/AMDI005 have an off by one bug
> in their function mask return.  This means that they will call entrance
> but not exit for matching functions.
>
> Other AMD systems with this HID should use the Microsoft generic UUID.
>
> AMD systems with uPEP HID AMDI006 should be using the Microsoft method.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Julian Sikorski <belegdol@gmail.com>


The series makes s2idle works on HP EliteBook 845 G8.

For the whole series,

Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

> ---
>   drivers/acpi/x86/s2idle.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 3f2a90648ec9..816bf2c34b7a 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -371,12 +371,27 @@ static int lps0_device_attach(struct acpi_device *adev,
>   		return 0;
>   
>   	if (acpi_s2idle_vendor_amd()) {
> +		/* AMD0004, AMDI0005:
> +		 * - Should use rev_id 0x0
> +		 * - function mask > 0x3: Should use AMD method, but has off by one bug
> +		 * - function mask = 0x3: Should use Microsoft method
> +		 * AMDI0006:
> +		 * - should use rev_id 0x0
> +		 * - function mask = 0x3: Should use Microsoft method
> +		 */
> +		const char *hid = acpi_device_hid(adev);
>   		rev_id = 0;
>   		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, rev_id,
>   					&lps0_dsm_guid_microsoft);
> +		if (lps0_dsm_func_mask > 0x3 && (!strcmp(hid, "AMD0004") ||
> +						 !strcmp(hid, "AMDI0005"))) {
> +			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 {
>   		rev_id = 1;
>   		lps0_dsm_func_mask = validate_dsm(adev->handle,
>

Re: [PATCH 1/5] ACPI: PM: s2idle: Use correct revision id

[Julian Sikorski]

W dniu 17.06.2021 o 18:42, Mario Limonciello pisze:
> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> 
> AMD spec mentions only revision 0. With this change,
> device constraint list is populated properly.
> 
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
> ---
>   drivers/acpi/x86/s2idle.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 2d7ddb8a8cb6..da27c1c45c9f 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -96,7 +96,7 @@ static void lpi_device_get_constraints_amd(void)
>   	int i, j, k;
>   
>   	out_obj = acpi_evaluate_dsm_typed(lps0_device_handle, &lps0_dsm_guid,
> -					  1, ACPI_LPS0_GET_DEVICE_CONSTRAINTS,
> +					  rev_id, ACPI_LPS0_GET_DEVICE_CONSTRAINTS,
>   					  NULL, ACPI_TYPE_PACKAGE);
>   
>   	if (!out_obj)
> 
No regressions against [1] on my ASUS UM425IA. Suspend and resume still 
work, provided other relevant patches are applied. I am testing on 
Fedora 5.12 kernel [2].

Tested-by: Julian Sikorski <belegdol@gmail.com>

[1] 
https://patchwork.kernel.org/project/linux-acpi/patch/20210317143842.786380-2-alexander.deucher@amd.com/
[2] 
https://gitlab.com/belegdol/kernel-ark/-/commits/fedora-5.12-s0ix-8-upstream-newnvme