ACPI: power: Save the last known state of each power resource

Currently, there are two ways to check the state of an ACPI power
resource and they may not be consistent with each other.  The first
one is to evaluate the power resource's _STA object and the other one
is to check its reference counter value.  However, on some systems
the value returned by _STA may not be consistent with the value of
the power resource's reference counter (for example, on some systems
it returns the same value every time for certain power resources).

Moreover, evaluating _STA is unnecessary overhead for a power
resource for which it has been evaluated already or whose state is
otherwise known, because either the _ON or the _OFF method has been
executed for it.

For this reason, save the state of each power resource in its
struct acpi_power_resource object and use the saved value whenever
its state needs to be checked, except when its stats is unknown, in
which case the _STA method is evaluated for it and the value
returned by that method is saved as the last known state of
the power resource.

Moreover, drop the power resource _STA method evaluation from
acpi_add_power_resource(), so as to avoid doing that unnecessarily
for power resources that will never be used.

Tested-by: Dave Olsthoorn <dave@bewaar.me>
Tested-by: Shujun Wang <wsj20369@163.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

1 files changed, 32 insertions, 18 deletions

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 106a5d37486c..6aeea7649852 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -53,6 +53,7 @@ struct acpi_power_resource {
 	u32 order;
 	unsigned int ref_count;
 	unsigned int users;
+	u8 state;
 	bool wakeup_enabled;
 	struct mutex resource_lock;
 	struct list_head dependents;
@@ -182,15 +183,12 @@ int acpi_extract_power_resources(union acpi_object *package, unsigned int start,
 	return err;
 }
 
-static int acpi_power_get_state(acpi_handle handle, u8 *state)
+static int __get_state(acpi_handle handle, u8 *state)
 {
 	acpi_status status = AE_OK;
 	unsigned long long sta = 0;
 	u8 cur_state;
 
-	if (!handle || !state)
-		return -EINVAL;
-
 	status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
@@ -204,6 +202,20 @@ static int acpi_power_get_state(acpi_handle handle, u8 *state)
 	return 0;
 }
 
+static int acpi_power_get_state(struct acpi_power_resource *resource, u8 *state)
+{
+	if (resource->state == ACPI_POWER_RESOURCE_STATE_UNKNOWN) {
+		int ret;
+
+		ret = __get_state(resource->device.handle, &resource->state);
+		if (ret)
+			return ret;
+	}
+
+	*state = resource->state;
+	return 0;
+}
+
 static int acpi_power_get_list_state(struct list_head *list, u8 *state)
 {
 	struct acpi_power_resource_entry *entry;
@@ -215,11 +227,10 @@ static int acpi_power_get_list_state(struct list_head *list, u8 *state)
 	/* The state of the list is 'on' IFF all resources are 'on'. */
 	list_for_each_entry(entry, list, node) {
 		struct acpi_power_resource *resource = entry->resource;
-		acpi_handle handle = resource->device.handle;
 		int result;
 
 		mutex_lock(&resource->resource_lock);
-		result = acpi_power_get_state(handle, &cur_state);
+		result = acpi_power_get_state(resource, &cur_state);
 		mutex_unlock(&resource->resource_lock);
 		if (result)
 			return result;
@@ -352,8 +363,12 @@ static int __acpi_power_on(struct acpi_power_resource *resource)
 	acpi_status status = AE_OK;
 
 	status = acpi_evaluate_object(resource->device.handle, "_ON", NULL, NULL);
-	if (ACPI_FAILURE(status))
+	if (ACPI_FAILURE(status)) {
+		resource->state = ACPI_POWER_RESOURCE_STATE_UNKNOWN;
 		return -ENODEV;
+	}
+
+	resource->state = ACPI_POWER_RESOURCE_STATE_ON;
 
 	pr_debug("Power resource [%s] turned on\n", resource->name);
 
@@ -405,8 +420,12 @@ static int __acpi_power_off(struct acpi_power_resource *resource)
 
 	status = acpi_evaluate_object(resource->device.handle, "_OFF",
 				      NULL, NULL);
-	if (ACPI_FAILURE(status))
+	if (ACPI_FAILURE(status)) {
+		resource->state = ACPI_POWER_RESOURCE_STATE_UNKNOWN;
 		return -ENODEV;
+	}
+
+	resource->state = ACPI_POWER_RESOURCE_STATE_OFF;
 
 	pr_debug("Power resource [%s] turned off\n", resource->name);
 
@@ -590,13 +609,12 @@ int acpi_power_wakeup_list_init(struct list_head *list, int *system_level_p)
 
 	list_for_each_entry(entry, list, node) {
 		struct acpi_power_resource *resource = entry->resource;
-		acpi_handle handle = resource->device.handle;
 		int result;
 		u8 state;
 
 		mutex_lock(&resource->resource_lock);
 
-		result = acpi_power_get_state(handle, &state);
+		result = acpi_power_get_state(resource, &state);
 		if (result) {
 			mutex_unlock(&resource->resource_lock);
 			return result;
@@ -920,7 +938,6 @@ struct acpi_device *acpi_add_power_resource(acpi_handle handle)
 	struct acpi_buffer buffer = { sizeof(acpi_object), &acpi_object };
 	acpi_status status;
 	int result;
-	u8 state;
 
 	acpi_bus_get_device(handle, &device);
 	if (device)
@@ -947,13 +964,9 @@ struct acpi_device *acpi_add_power_resource(acpi_handle handle)
 
 	resource->system_level = acpi_object.power_resource.system_level;
 	resource->order = acpi_object.power_resource.resource_order;
+	resource->state = ACPI_POWER_RESOURCE_STATE_UNKNOWN;
 
-	result = acpi_power_get_state(handle, &state);
-	if (result)
-		goto err;
-
-	pr_info("%s [%s] (%s)\n", acpi_device_name(device),
-		acpi_device_bid(device), state ? "on" : "off");
+	pr_info("%s [%s]\n", acpi_device_name(device), acpi_device_bid(device));
 
 	device->flags.match_driver = true;
 	result = acpi_device_add(device, acpi_release_power_resource);
@@ -985,7 +998,8 @@ void acpi_resume_power_resources(void)
 
 		mutex_lock(&resource->resource_lock);
 
-		result = acpi_power_get_state(resource->device.handle, &state);
+		resource->state = ACPI_POWER_RESOURCE_STATE_UNKNOWN;
+		result = acpi_power_get_state(resource, &state);
 		if (result) {
 			mutex_unlock(&resource->resource_lock);
 			continue;