From owner-freebsd-acpi@FreeBSD.ORG Wed Nov 24 17:49:24 2004 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 407B616A4CE for ; Wed, 24 Nov 2004 17:49:24 +0000 (GMT) Received: from ylpvm43.prodigy.net (ylpvm43-ext.prodigy.net [207.115.57.74]) by mx1.FreeBSD.org (Postfix) with ESMTP id B657243D41 for ; Wed, 24 Nov 2004 17:49:23 +0000 (GMT) (envelope-from nate@root.org) Received: from [10.0.0.34] (adsl-67-119-74-222.dsl.sntc01.pacbell.net [67.119.74.222])iAOHnNp8012158; Wed, 24 Nov 2004 12:49:23 -0500 Message-ID: <41A4C99E.8050507@root.org> Date: Wed, 24 Nov 2004 09:49:18 -0800 From: Nate Lawson User-Agent: Mozilla Thunderbird 0.7.3 (X11/20040901) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kevin Oberman References: <20041124173304.3D7A15D07@ptavv.es.net> In-Reply-To: <20041124173304.3D7A15D07@ptavv.es.net> Content-Type: multipart/mixed; boundary="------------010606030407040208010108" cc: acpi@FreeBSD.org Subject: Re: PATCH: power down acpi and pci devices in suspend/resume X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Nov 2004 17:49:24 -0000 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 #include +#include "pci_if.h" +#include +#include + 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 +#include +#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--