Date: Mon, 29 Nov 2004 11:15:05 -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: <41AB7539.1090305@root.org> In-Reply-To: <20041124232339.E1B605D07@ptavv.es.net> References: <20041124232339.E1B605D07@ptavv.es.net>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------000307060008090406000508
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Kevin Oberman wrote:
>
> The new patch removed the annoying "bad Vcc request" messages, but
> that's all it improved. With the new patch I still lose cbb1 and
> anything connected to it. I see no real difference in the log other than
> the disappearance of the Vcc messages, but that is a good thing.
>
> If I set debug.suspend_power to '0', everything works as it did
> before. All PCI and CardBus devices seem to work fine after resume.
Ok, I've revved it to only power down type 0 PCI (i.e. normal) devices
so it shouldn't touch your cbb bridge. Please try the attached patch.
Note that there are now two separate tunables/sysctls to disable
powerstates:
hw.pci.do_powerstate=1
debug.acpi.do_powerstate=1
This way you can disable PCI and ACPI power separately for debugging.
Once things are stabilized, these will go away.
-Nate
--------------000307060008090406000508
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 29 Nov 2004 19:13:26 -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,12 @@
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. */
+static int acpi_do_powerstate = 1;
+TUNABLE_INT("debug.acpi.do_powerstate", &acpi_do_powerstate);
+SYSCTL_INT(_debug_acpi, OID_AUTO, do_powerstate, CTLFLAG_RW,
+ &acpi_do_powerstate, 1, "Turn off devices when suspending.");
+
/*
* 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 +592,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_do_powerstate)
+ 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_do_powerstate)
+ 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 +712,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 +1169,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 +1294,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/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 29 Nov 2004 18:46:49 -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);
@@ -169,7 +173,7 @@
SYSCTL_NODE(_hw, OID_AUTO, pci, CTLFLAG_RD, 0, "PCI bus tuning parameters");
static int pci_enable_io_modes = 1;
-TUNABLE_INT("hw.pci.enable_io_modes", (int *)&pci_enable_io_modes);
+TUNABLE_INT("hw.pci.enable_io_modes", &pci_enable_io_modes);
SYSCTL_INT(_hw_pci, OID_AUTO, enable_io_modes, CTLFLAG_RW,
&pci_enable_io_modes, 1,
"Enable I/O and memory bits in the config register. Some BIOSes do not\n\
@@ -177,7 +181,7 @@
are some peripherals that this causes problems with.");
static int pci_do_powerstate = 1;
-TUNABLE_INT("hw.pci.do_powerstate", (int *)&pci_do_powerstate);
+TUNABLE_INT("hw.pci.do_powerstate", &pci_do_powerstate);
SYSCTL_INT(_hw_pci, OID_AUTO, do_powerstate, CTLFLAG_RW,
&pci_do_powerstate, 1,
"Power down devices into D3 state when no driver attaches to them.\n\
@@ -1016,43 +1020,78 @@
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 (pci_do_powerstate)
+ 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. Skip
+ * children who aren't attached since they are powered down
+ * separately. Only manage type 0 devices for now.
+ */
+ for (i = 0; acpi_dev && i < numdevs; i++) {
+ child = devlist[i];
+ dinfo = (struct pci_devinfo *) device_get_ivars(child);
+ if (device_is_attached(child) && dinfo->cfg.hdrtype == 0) {
+ 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 (pci_do_powerstate)
+ 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. Only manage type 0 devices for now.
+ */
child = devlist[i];
dinfo = (struct pci_devinfo *) device_get_ivars(child);
+ if (acpi_dev && device_is_attached(child) &&
+ dinfo->cfg.hdrtype == 0) {
+ 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. */
pci_cfg_restore(child, dinfo);
}
free(devlist, M_TEMP);
--------------000307060008090406000508--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41AB7539.1090305>