Date: Wed, 24 Nov 2004 09:49:18 -0800 From: Nate Lawson <nate@root.org> To: Kevin Oberman <oberman@es.net> Cc: acpi@FreeBSD.org Subject: Re: PATCH: power down acpi and pci devices in suspend/resume Message-ID: <41A4C99E.8050507@root.org> In-Reply-To: <20041124173304.3D7A15D07@ptavv.es.net> References: <20041124173304.3D7A15D07@ptavv.es.net>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------010606030407040208010108
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Kevin Oberman wrote:
> I have tried the new set of ACPI power patches and they are better. Now
> the system almost works after resume. Only the cbb fails:
> cbb0: bad Vcc request. ctrl=0xffffff88, status=0xffffffff
> cbb_power: 0V
> tdkphy0: detached
Apologies, I just found what was causing this. My patch to perform
suspending before powering down devices didn't get merged with this tree
where I was implementing powerstates. I fixed this and unified pci/acpi
power on suspend behavior under the tunable/sysctl "debug.suspend_power".
Please test the attached patch. If it works well, I'll commit it as
shown to get testing in -current. If it causes trouble, the default for
debug.suspend_power can be set to 0.
-Nate
--------------010606030407040208010108
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 17:41:34 -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,9 @@
static int acpi_serialize_methods;
TUNABLE_INT("hw.acpi.serialize_methods", &acpi_serialize_methods);
+/* Power devices off and on in suspend and resume. XXX Remove once tested. */
+extern int do_suspend_power;
+
/*
* 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 +589,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) && do_suspend_power)
+ 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) && do_suspend_power)
+ 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 +709,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 +1166,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 +1291,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 24 Nov 2004 17:40:58 -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);
@@ -183,6 +187,11 @@
"Power down devices into D3 state when no driver attaches to them.\n\
Otherwise, leave the device in D0 state when no driver attaches.");
+int do_suspend_power = 1;
+TUNABLE_INT("debug.suspend_power", &do_suspend_power);
+SYSCTL_INT(_debug, OID_AUTO, suspend_power, CTLFLAG_RW,
+ &do_suspend_power, 1, "Power down devices into D3 state in suspend.");
+
/* Find a device_t by bus/slot/function */
device_t
@@ -1016,42 +1025,71 @@
int
pci_suspend(device_t dev)
{
- int numdevs;
- device_t *devlist;
- device_t child;
+ int dstate, error, 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 = NULL;
+ if (do_suspend_power)
+ 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);
}
+
+ /* Suspend devices before potentially powering them down. */
+ error = bus_generic_suspend(dev);
+ if (error)
+ return (error);
+
+ /*
+ * 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.
+ */
+ for (i = 0; acpi_dev && i < numdevs; i++) {
+ child = devlist[i];
+ 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));
+ return (0);
}
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 = NULL;
+ if (do_suspend_power)
+ 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) {
+ 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);
}
--------------010606030407040208010108--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41A4C99E.8050507>