The branch main has been updated by obiwac:
URL:
https://cgit.FreeBSD.org/src/commit/?id=a13f28d57ecfd136ce73493659c28a47fa1a4b9f
commit a13f28d57ecfd136ce73493659c28a47fa1a4b9f
Author: Aymeric Wibo <obi...@freebsd.org>
AuthorDate: 2025-08-18 14:00:59 +0000
Commit: Aymeric Wibo <obi...@freebsd.org>
CommitDate: 2025-08-18 14:01:06 +0000
acpi_powerres: Fix turning off power resources on first D-state switch
The power resource dependencies for each `_PRx` object are discovered
and cached in `ac_prx` on the power consumer struct
(`struct acpi_powerconsumer`) when a power consumer is registered. This
is done in `acpi_pwr_get_power_resources`. ACPI guarantees these `_PRx`
objects will evaluate to the same thing each time they are called.
This discovery process also registers those power resources, which were
previously only registered when they were referenced by the relevant
`_PRx` object for the target D-state when switching. This meant that
the first D-state switch for a power consumer would not turn off any
power resources as they wouldn't have been registered yet. This change
fixes this.
`ac_prx` will be used by subsequent patches.
Reviewed by: markj, imp, jrm (mentor)
Approved by: markj, jrm (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D48385
---
sys/dev/acpica/acpi_powerres.c | 110 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 105 insertions(+), 5 deletions(-)
diff --git a/sys/dev/acpica/acpi_powerres.c b/sys/dev/acpica/acpi_powerres.c
index 29d1690f1bdd..0baa5c595470 100644
--- a/sys/dev/acpica/acpi_powerres.c
+++ b/sys/dev/acpica/acpi_powerres.c
@@ -76,6 +76,13 @@ struct acpi_powerconsumer {
/* Device which is powered */
ACPI_HANDLE ac_consumer;
int ac_state;
+
+ struct {
+ bool prx_has;
+ size_t prx_count;
+ ACPI_HANDLE *prx_deps;
+ } ac_prx[ACPI_D_STATE_COUNT];
+
TAILQ_ENTRY(acpi_powerconsumer) ac_link;
TAILQ_HEAD(,acpi_powerreference) ac_references;
};
@@ -96,9 +103,7 @@ static TAILQ_HEAD(acpi_powerconsumer_list,
acpi_powerconsumer)
ACPI_SERIAL_DECL(powerres, "ACPI power resources");
static ACPI_STATUS acpi_pwr_register_consumer(ACPI_HANDLE consumer);
-#ifdef notyet
static ACPI_STATUS acpi_pwr_deregister_consumer(ACPI_HANDLE consumer);
-#endif /* notyet */
static ACPI_STATUS acpi_pwr_register_resource(ACPI_HANDLE res);
#ifdef notyet
static ACPI_STATUS acpi_pwr_deregister_resource(ACPI_HANDLE res);
@@ -221,6 +226,84 @@ acpi_pwr_deregister_resource(ACPI_HANDLE res)
}
#endif /* notyet */
+/*
+ * Evaluate the _PRx (power resources each D-state depends on). This also
+ * populates the acpi_powerresources queue with the power resources discovered
+ * during this step.
+ *
+ * ACPI 7.3.8 - 7.3.11 guarantee that _PRx will return the same data each
+ * time they are evaluated.
+ *
+ * If this function fails, acpi_pwr_deregister_consumer() must be called on the
+ * power consumer to free already allocated memory.
+ */
+static ACPI_STATUS
+acpi_pwr_get_power_resources(ACPI_HANDLE consumer, struct acpi_powerconsumer
*pc)
+{
+ ACPI_INTEGER status;
+ ACPI_STRING reslist_name;
+ ACPI_HANDLE reslist_handle;
+ ACPI_STRING reslist_names[] = {"_PR0", "_PR1", "_PR2",
"_PR3"};
+ ACPI_BUFFER reslist;
+ ACPI_OBJECT *reslist_object;
+ ACPI_OBJECT *dep;
+ ACPI_HANDLE *res;
+
+ ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
+ ACPI_SERIAL_ASSERT(powerres);
+
+ MPASS(consumer != NULL);
+
+ for (int state = ACPI_STATE_D0; state <= ACPI_STATE_D3_HOT; state++) {
+ pc->ac_prx[state].prx_has = false;
+ pc->ac_prx[state].prx_count = 0;
+ pc->ac_prx[state].prx_deps = NULL;
+
+ reslist_name = reslist_names[state - ACPI_STATE_D0];
+ if (ACPI_FAILURE(AcpiGetHandle(consumer, reslist_name,
&reslist_handle)))
+ continue;
+
+ reslist.Pointer = NULL;
+ reslist.Length = ACPI_ALLOCATE_BUFFER;
+ status = AcpiEvaluateObjectTyped(reslist_handle, NULL, NULL, &reslist,
+ ACPI_TYPE_PACKAGE);
+ if (ACPI_FAILURE(status) || reslist.Pointer == NULL)
+ /*
+ * ACPI_ALLOCATE_BUFFER entails everything will be freed on error
+ * by AcpiEvaluateObjectTyped.
+ */
+ continue;
+
+ reslist_object = (ACPI_OBJECT *)reslist.Pointer;
+ pc->ac_prx[state].prx_has = true;
+ pc->ac_prx[state].prx_count = reslist_object->Package.Count;
+
+ if (reslist_object->Package.Count == 0) {
+ AcpiOsFree(reslist_object);
+ continue;
+ }
+
+ pc->ac_prx[state].prx_deps = mallocarray(pc->ac_prx[state].prx_count,
+ sizeof(*pc->ac_prx[state].prx_deps), M_ACPIPWR, M_NOWAIT);
+ if (pc->ac_prx[state].prx_deps == NULL) {
+ AcpiOsFree(reslist_object);
+ return_ACPI_STATUS (AE_NO_MEMORY);
+ }
+
+ for (size_t i = 0; i < reslist_object->Package.Count; i++) {
+ dep = &reslist_object->Package.Elements[i];
+ res = dep->Reference.Handle;
+ pc->ac_prx[state].prx_deps[i] = res;
+
+ /* It's fine to attempt to register the same resource twice. */
+ acpi_pwr_register_resource(res);
+ }
+ AcpiOsFree(reslist_object);
+ }
+
+ return_ACPI_STATUS (AE_OK);
+}
+
/*
* Register a power consumer.
*
@@ -229,6 +312,7 @@ acpi_pwr_deregister_resource(ACPI_HANDLE res)
static ACPI_STATUS
acpi_pwr_register_consumer(ACPI_HANDLE consumer)
{
+ ACPI_INTEGER status;
struct acpi_powerconsumer *pc;
ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
@@ -239,12 +323,27 @@ acpi_pwr_register_consumer(ACPI_HANDLE consumer)
return_ACPI_STATUS (AE_OK);
/* Allocate a new power consumer */
- if ((pc = malloc(sizeof(*pc), M_ACPIPWR, M_NOWAIT)) == NULL)
+ if ((pc = malloc(sizeof(*pc), M_ACPIPWR, M_NOWAIT | M_ZERO)) == NULL)
return_ACPI_STATUS (AE_NO_MEMORY);
TAILQ_INSERT_HEAD(&acpi_powerconsumers, pc, ac_link);
TAILQ_INIT(&pc->ac_references);
pc->ac_consumer = consumer;
+ /*
+ * Get all its power resource dependencies, if it has _PRx. We do this now
+ * as an opportunity to populate the acpi_powerresources queue.
+ *
+ * If this fails, immediately deregister it.
+ */
+ status = acpi_pwr_get_power_resources(consumer, pc);
+ if (ACPI_FAILURE(status)) {
+ ACPI_DEBUG_PRINT((ACPI_DB_OBJECTS,
+ "failed to get power resources for %s\n",
+ acpi_name(consumer)));
+ acpi_pwr_deregister_consumer(consumer);
+ return_ACPI_STATUS (status);
+ }
+
/* XXX we should try to find its current state */
pc->ac_state = ACPI_STATE_UNKNOWN;
@@ -254,7 +353,6 @@ acpi_pwr_register_consumer(ACPI_HANDLE consumer)
return_ACPI_STATUS (AE_OK);
}
-#ifdef notyet
/*
* Deregister a power consumer.
*
@@ -279,6 +377,9 @@ acpi_pwr_deregister_consumer(ACPI_HANDLE consumer)
/* Pull the consumer off the list and free it */
TAILQ_REMOVE(&acpi_powerconsumers, pc, ac_link);
+ for (size_t i = 0; i < sizeof(pc->ac_prx) / sizeof(*pc->ac_prx); i++)
+ if (pc->ac_prx[i].prx_deps != NULL)
+ free(pc->ac_prx[i].prx_deps, M_ACPIPWR);
free(pc, M_ACPIPWR);
ACPI_DEBUG_PRINT((ACPI_DB_OBJECTS, "deregistered power consumer %s\n",
@@ -286,7 +387,6 @@ acpi_pwr_deregister_consumer(ACPI_HANDLE consumer)
return_ACPI_STATUS (AE_OK);
}
-#endif /* notyet */
/*
* Set a power consumer to a particular power state.
