Date: Tue, 23 Nov 2004 23:42:33 -0800 From: Nate Lawson <nate@root.org> To: acpi@FreeBSD.org Subject: Re: PATCH: power down acpi and pci devices in suspend/resume Message-ID: <41A43B69.60901@root.org> In-Reply-To: <20041121233202.8AEB95D04@ptavv.es.net> References: <20041121233202.8AEB95D04@ptavv.es.net>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------000103000906030200070407
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Nate Lawson wrote:
>>Date: Fri, 19 Nov 2004 23:52:13 -0800
>>From: Nate Lawson <nate@root.org>
>>The attached patch implements setting power states for ACPI (i.e. ISA)
>>and PCI devices in the suspend/resume path. This may help with some
>>problems; it's quite likely it may introduce problems. That's why I'd
>>like it tested. If you have a system that suspends/resumes ok or that
>>fails, please try it. The likely failure case is a hang in suspend or
>>resume or a device that doesn't work afterwords. It's pretty
>>heavy-handed, only avoiding changing power for serial ports since those
>>are known to cause a hang (which can possibly be fixed by making
>>sio/uart more aware of power states.) I suspect devices like PCI
>>bridges may have problems with power changes.
>>
>>If you have problems, please let me know the info it prints before the
>>hang so I can figure out what the problem device is.
Ok, here's another cut of the patch. It should fix the APM case (by not
tweaking power states for it) among other things. It has a tunable
(defaulting to "on" or 1) where you can turn off the acpi part of things
to see if that helps/hurts: hw.acpi.sleep_power
-Nate
--------------000103000906030200070407
Content-Type: text/plain;
name="acpi_pwr.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="acpi_pwr.diff"
Index: sys/dev/acpica/acpi.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/acpica/acpi.c,v
retrieving revision 1.193
diff -u -r1.193 acpi.c
--- sys/dev/acpica/acpi.c 13 Oct 2004 07:29:29 -0000 1.193
+++ sys/dev/acpica/acpi.c 24 Nov 2004 07:35:54 -0000
@@ -59,6 +59,10 @@
#include <dev/acpica/acpiio.h>
#include <contrib/dev/acpica/acnamesp.h>
+#include "pci_if.h"
+#include <dev/pci/pcivar.h>
+#include <dev/pci/pci_private.h>
+
MALLOC_DEFINE(M_ACPIDEV, "acpidev", "ACPI devices");
/* Hooks for the ACPI CA debugging infrastructure */
@@ -87,10 +91,13 @@
static void acpi_identify(driver_t *driver, device_t parent);
static int acpi_probe(device_t dev);
static int acpi_attach(device_t dev);
+static int acpi_suspend(device_t dev);
+static int acpi_resume(device_t dev);
static int acpi_shutdown(device_t dev);
static device_t acpi_add_child(device_t bus, int order, const char *name,
int unit);
static int acpi_print_child(device_t bus, device_t child);
+static void acpi_probe_nomatch(device_t bus, device_t child);
static int acpi_read_ivar(device_t dev, device_t child, int index,
uintptr_t *result);
static int acpi_write_ivar(device_t dev, device_t child, int index,
@@ -110,10 +117,14 @@
static ACPI_STATUS acpi_device_eval_obj(device_t bus, device_t dev,
ACPI_STRING pathname, ACPI_OBJECT_LIST *parameters,
ACPI_BUFFER *ret);
+static int acpi_device_pwr_for_sleep(device_t bus, device_t dev,
+ int *dstate);
static ACPI_STATUS acpi_device_scan_cb(ACPI_HANDLE h, UINT32 level,
void *context, void **retval);
static ACPI_STATUS acpi_device_scan_children(device_t bus, device_t dev,
int max_depth, acpi_scan_cb_t user_fn, void *arg);
+static int acpi_set_powerstate_method(device_t bus, device_t child,
+ int state);
static int acpi_isa_pnp_probe(device_t bus, device_t child,
struct isa_pnp_id *ids);
static void acpi_probe_children(device_t bus);
@@ -145,12 +156,13 @@
DEVMETHOD(device_attach, acpi_attach),
DEVMETHOD(device_shutdown, acpi_shutdown),
DEVMETHOD(device_detach, bus_generic_detach),
- DEVMETHOD(device_suspend, bus_generic_suspend),
- DEVMETHOD(device_resume, bus_generic_resume),
+ DEVMETHOD(device_suspend, acpi_suspend),
+ DEVMETHOD(device_resume, acpi_resume),
/* Bus interface */
DEVMETHOD(bus_add_child, acpi_add_child),
DEVMETHOD(bus_print_child, acpi_print_child),
+ DEVMETHOD(bus_probe_nomatch, acpi_probe_nomatch),
DEVMETHOD(bus_read_ivar, acpi_read_ivar),
DEVMETHOD(bus_write_ivar, acpi_write_ivar),
DEVMETHOD(bus_get_resource_list, acpi_get_rlist),
@@ -169,8 +181,12 @@
/* ACPI bus */
DEVMETHOD(acpi_id_probe, acpi_device_id_probe),
DEVMETHOD(acpi_evaluate_object, acpi_device_eval_obj),
+ DEVMETHOD(acpi_pwr_for_sleep, acpi_device_pwr_for_sleep),
DEVMETHOD(acpi_scan_children, acpi_device_scan_children),
+ /* PCI emulation */
+ DEVMETHOD(pci_set_powerstate, acpi_set_powerstate_method),
+
/* ISA emulation */
DEVMETHOD(isa_pnp_probe, acpi_isa_pnp_probe),
@@ -212,6 +228,10 @@
static int acpi_serialize_methods;
TUNABLE_INT("hw.acpi.serialize_methods", &acpi_serialize_methods);
+/* Power devices off and on in suspend and resume. */
+static int acpi_pwr_sleep = 1;
+TUNABLE_INT("hw.acpi.sleep_power", &acpi_pwr_sleep);
+
/*
* ACPI can only be loaded as a module by the loader; activating it after
* system bootstrap time is not useful, and can be fatal to the system.
@@ -570,6 +590,72 @@
}
static int
+acpi_suspend(device_t dev)
+{
+ struct acpi_softc *sc;
+ device_t child, *devlist;
+ int error, i, numdevs, pstate;
+
+ /* First give child devices a chance to suspend. */
+ error = bus_generic_suspend(dev);
+ if (error)
+ return (error);
+
+ /*
+ * Now, set them into the appropriate power state, usually D3. If the
+ * device has an _SxD method for the next sleep state, use that power
+ * state instead.
+ */
+ sc = device_get_softc(dev);
+ device_get_children(dev, &devlist, &numdevs);
+ for (i = 0; i < numdevs; i++) {
+ /* If the device is not attached, we've powered it down elsewhere. */
+ child = devlist[i];
+ if (!device_is_attached(child))
+ continue;
+
+ /*
+ * Default to D3 for all sleep states. The _SxD method is optional
+ * so set the powerstate even if it's absent.
+ */
+ pstate = PCI_POWERSTATE_D3;
+ error = acpi_device_pwr_for_sleep(device_get_parent(child),
+ child, &pstate);
+ if ((error == 0 || error == ESRCH) && acpi_pwr_sleep)
+ pci_set_powerstate(child, pstate);
+ }
+ free(devlist, M_TEMP);
+ error = 0;
+
+ return (error);
+}
+
+static int
+acpi_resume(device_t dev)
+{
+ ACPI_HANDLE handle;
+ int i, numdevs;
+ device_t child, *devlist;
+
+ /*
+ * Put all devices in D0 before resuming them. Call _S0D on each one
+ * since some systems expect this.
+ */
+ device_get_children(dev, &devlist, &numdevs);
+ for (i = 0; i < numdevs; i++) {
+ child = devlist[i];
+ handle = acpi_get_handle(child);
+ if (handle)
+ AcpiEvaluateObject(handle, "_S0D", NULL, NULL);
+ if (device_is_attached(child) && acpi_pwr_sleep)
+ pci_set_powerstate(child, PCI_POWERSTATE_D0);
+ }
+ free(devlist, M_TEMP);
+
+ return (bus_generic_resume(dev));
+}
+
+static int
acpi_shutdown(device_t dev)
{
@@ -624,6 +710,23 @@
return (retval);
}
+static void
+acpi_probe_nomatch(device_t bus, device_t child)
+{
+
+ /*
+ * If this device is an ACPI child but no one claimed it, attempt
+ * to power it off. We'll power it back up when a driver is added.
+ *
+ * XXX Disabled for now since many necessary devices (like fdc and
+ * ATA) don't claim the devices we created for them but still expect
+ * them to be powered up.
+ */
+#if 0
+ pci_set_powerstate(child, PCI_POWERSTATE_D3);
+#endif
+}
+
/* Location hint for devctl(8) */
static int
acpi_child_location_str_method(device_t cbdev, device_t child, char *buf,
@@ -1064,6 +1167,57 @@
return (AcpiEvaluateObject(h, pathname, parameters, ret));
}
+static int
+acpi_device_pwr_for_sleep(device_t bus, device_t dev, int *dstate)
+{
+ struct acpi_softc *sc;
+ ACPI_HANDLE handle;
+ ACPI_STATUS status;
+ char sxd[8];
+ int error;
+
+ sc = device_get_softc(bus);
+ handle = acpi_get_handle(dev);
+
+ /*
+ * XXX If we find these devices, don't try to power them down.
+ * The serial and IRDA ports on my T23 hang the system when
+ * set to D3 and it appears that such legacy devices may
+ * need special handling in their drivers.
+ */
+ if (handle == NULL ||
+ acpi_MatchHid(handle, "PNP0500") ||
+ acpi_MatchHid(handle, "PNP0501") ||
+ acpi_MatchHid(handle, "PNP0502") ||
+ acpi_MatchHid(handle, "PNP0510") ||
+ acpi_MatchHid(handle, "PNP0511"))
+ return (ENXIO);
+
+ /*
+ * Override next state with the value from _SxD, if present. If no
+ * dstate argument was provided, don't fetch the return value.
+ */
+ snprintf(sxd, sizeof(sxd), "_S%dD", sc->acpi_sstate);
+ if (dstate)
+ status = acpi_GetInteger(handle, sxd, dstate);
+ else
+ status = AcpiEvaluateObject(handle, sxd, NULL, NULL);
+
+ switch (status) {
+ case AE_OK:
+ error = 0;
+ break;
+ case AE_NOT_FOUND:
+ error = ESRCH;
+ break;
+ default:
+ error = ENXIO;
+ break;
+ }
+
+ return (error);
+}
+
/* Callback arg for our implementation of walking the namespace. */
struct acpi_device_scan_ctx {
acpi_scan_cb_t user_fn;
@@ -1138,6 +1292,34 @@
acpi_device_scan_cb, &ctx, NULL));
}
+/*
+ * Even though ACPI devices are not PCI, we use the PCI approach for setting
+ * device power states since it's close enough to ACPI.
+ */
+static int
+acpi_set_powerstate_method(device_t bus, device_t child, int state)
+{
+ ACPI_HANDLE h;
+ ACPI_STATUS status;
+ int error;
+
+ error = 0;
+ h = acpi_get_handle(child);
+ if (state < ACPI_STATE_D0 || state > ACPI_STATE_D3)
+ return (EINVAL);
+ if (h == NULL)
+ return (0);
+
+ /* Ignore errors if the power methods aren't present. */
+ status = acpi_pwr_switch_consumer(h, state);
+ if (ACPI_FAILURE(status) && status != AE_NOT_FOUND
+ && status != AE_BAD_PARAMETER)
+ device_printf(bus, "failed to set ACPI power state D%d on %s: %s\n",
+ state, acpi_name(h), AcpiFormatException(status));
+
+ return (error);
+}
+
static int
acpi_isa_pnp_probe(device_t bus, device_t child, struct isa_pnp_id *ids)
{
Index: sys/dev/acpica/acpi_if.m
===================================================================
RCS file: /home/ncvs/src/sys/dev/acpica/acpi_if.m,v
retrieving revision 1.2
diff -u -r1.2 acpi_if.m
--- sys/dev/acpica/acpi_if.m 15 Jul 2004 16:29:08 -0000 1.2
+++ sys/dev/acpica/acpi_if.m 20 Nov 2004 03:12:35 -0000
@@ -109,6 +109,26 @@
};
#
+# Get the highest power state (D0-D3) that is usable for a device when
+# suspending/resuming. If a bus calls this when suspending a device, it
+# must also call it when resuming.
+#
+# device_t bus: parent bus for the device
+#
+# device_t dev: check this device's appropriate power state
+#
+# int *dstate: if successful, contains the highest valid sleep state
+#
+# Returns: 0 on success, ESRCH if device has no special state, or
+# some other error value.
+#
+METHOD int pwr_for_sleep {
+ device_t bus;
+ device_t dev;
+ int *dstate;
+};
+
+#
# Rescan a subtree and optionally reattach devices to handles. Users
# specify a callback that is called for each ACPI_HANDLE of type Device
# that is a child of "dev".
Index: sys/dev/acpica/acpi_pci.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/acpica/acpi_pci.c,v
retrieving revision 1.24
diff -u -r1.24 acpi_pci.c
--- sys/dev/acpica/acpi_pci.c 22 Sep 2004 15:46:16 -0000 1.24
+++ sys/dev/acpica/acpi_pci.c 22 Nov 2004 17:54:25 -0000
@@ -59,6 +59,12 @@
ACPI_SERIAL_DECL(pci_powerstate, "ACPI PCI power methods");
+/* Be sure that ACPI and PCI power states are equivalent. */
+CTASSERT(ACPI_STATE_D0 == PCI_POWERSTATE_D0);
+CTASSERT(ACPI_STATE_D1 == PCI_POWERSTATE_D1);
+CTASSERT(ACPI_STATE_D2 == PCI_POWERSTATE_D2);
+CTASSERT(ACPI_STATE_D3 == PCI_POWERSTATE_D3);
+
static int acpi_pci_attach(device_t dev);
static int acpi_pci_child_location_str_method(device_t cbdev,
device_t child, char *buf, size_t buflen);
@@ -183,25 +189,11 @@
{
ACPI_HANDLE h;
ACPI_STATUS status;
- int acpi_state, old_state, error;
+ int old_state, error;
error = 0;
- switch (state) {
- case PCI_POWERSTATE_D0:
- acpi_state = ACPI_STATE_D0;
- break;
- case PCI_POWERSTATE_D1:
- acpi_state = ACPI_STATE_D1;
- break;
- case PCI_POWERSTATE_D2:
- acpi_state = ACPI_STATE_D2;
- break;
- case PCI_POWERSTATE_D3:
- acpi_state = ACPI_STATE_D3;
- break;
- default:
+ if (state < ACPI_STATE_D0 || state > ACPI_STATE_D3)
return (EINVAL);
- }
/*
* We set the state using PCI Power Management outside of setting
@@ -220,11 +212,11 @@
goto out;
}
h = acpi_get_handle(child);
- status = acpi_pwr_switch_consumer(h, acpi_state);
+ status = acpi_pwr_switch_consumer(h, state);
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
device_printf(dev,
"Failed to set ACPI power state D%d on %s: %s\n",
- acpi_state, acpi_name(h), AcpiFormatException(status));
+ state, acpi_name(h), AcpiFormatException(status));
if (old_state > state)
error = pci_set_powerstate_method(dev, child, state);
Index: sys/dev/pci/pci.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/pci/pci.c,v
retrieving revision 1.268
diff -u -r1.268 pci.c
--- sys/dev/pci/pci.c 10 Nov 2004 00:41:39 -0000 1.268
+++ sys/dev/pci/pci.c 22 Nov 2004 17:30:44 -0000
@@ -60,6 +60,10 @@
#include "pcib_if.h"
#include "pci_if.h"
+#include <contrib/dev/acpica/acpi.h>
+#include <dev/acpica/acpivar.h>
+#include "acpi_if.h"
+
static uint32_t pci_mapbase(unsigned mapreg);
static int pci_maptype(unsigned mapreg);
static int pci_mapsize(unsigned testval);
@@ -1016,22 +1020,31 @@
int
pci_suspend(device_t dev)
{
- int numdevs;
- device_t *devlist;
- device_t child;
+ int dstate, i, numdevs;
+ device_t acpi_dev, child, *devlist;
struct pci_devinfo *dinfo;
- int i;
/*
- * Save the pci configuration space for each child. We don't need
- * to do this, unless the BIOS suspend code powers down the bus and
- * the devices on the bus.
+ * Save the PCI configuration space for each child and set the
+ * device in the appropriate power state for this sleep state.
*/
+ acpi_dev = devclass_get_device(devclass_find("acpi"), 0);
device_get_children(dev, &devlist, &numdevs);
for (i = 0; i < numdevs; i++) {
child = devlist[i];
dinfo = (struct pci_devinfo *) device_get_ivars(child);
pci_cfg_save(child, dinfo, 0);
+
+ /*
+ * Always set the device to D3. If ACPI suggests a different
+ * power state, use it instead. If ACPI is not present, the
+ * firmware is responsible for managing device power.
+ */
+ if (acpi_dev != NULL) {
+ dstate = PCI_POWERSTATE_D3;
+ ACPI_PWR_FOR_SLEEP(acpi_dev, child, &dstate);
+ pci_set_powerstate(child, dstate);
+ }
}
free(devlist, M_TEMP);
return (bus_generic_suspend(dev));
@@ -1040,18 +1053,28 @@
int
pci_resume(device_t dev)
{
- int numdevs;
- device_t *devlist;
- device_t child;
+ int i, numdevs;
+ device_t acpi_dev, child, *devlist;
struct pci_devinfo *dinfo;
- int i;
/*
- * Restore the pci configuration space for each child.
+ * Set each child to D0 and restore its PCI configuration space.
*/
+ acpi_dev = devclass_get_device(devclass_find("acpi"), 0);
device_get_children(dev, &devlist, &numdevs);
for (i = 0; i < numdevs; i++) {
+ /*
+ * Notify ACPI we're going to D0 but ignore the result.
+ * If ACPI is not present, the firmware is responsible for
+ * managing device power.
+ */
child = devlist[i];
+ if (acpi_dev != NULL) {
+ ACPI_PWR_FOR_SLEEP(acpi_dev, child, NULL);
+ pci_set_powerstate(child, PCI_POWERSTATE_D0);
+ }
+
+ /* Now the device is powered up, restore its config space. */
dinfo = (struct pci_devinfo *) device_get_ivars(child);
pci_cfg_restore(child, dinfo);
}
--------------000103000906030200070407--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41A43B69.60901>