From owner-freebsd-acpi@FreeBSD.ORG Fri Mar 7 14:19:04 2008 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 952851065687 for ; Fri, 7 Mar 2008 14:19:04 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 803528FC25 for ; Fri, 7 Mar 2008 14:19:04 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from zion.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by elvis.mu.org (Postfix) with ESMTP id C5F481A4D80; Fri, 7 Mar 2008 06:18:33 -0800 (PST) From: John Baldwin To: freebsd-acpi@freebsd.org Date: Fri, 7 Mar 2008 07:55:26 -0500 User-Agent: KMail/1.9.7 References: <1204845766.1189@thinkpad.local> <47D0C917.6020904@root.org> <1204872781.815@thinkpad.local> In-Reply-To: <1204872781.815@thinkpad.local> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200803070755.26418.jhb@freebsd.org> Cc: "Jim Eberle " Subject: Re: ichss makes ThinkPad R31 toasty X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Mar 2008 14:19:04 -0000 On Friday 07 March 2008 01:53:00 am Jim Eberle wrote: > > Jim Eberle wrote: > > > ... > > > Either the temperature readings are wrong (unlikely), or simply > > > enabling SpeedStep ICH is making the machine into a toaster. :) > > > ... > > > Obviously, w/o ichss (or est or acpi_perf which aren't supported by > > > this hardware) there are no ".freq/.freq_levels" sysctls, so cpufreq > > > won't run. > > > > Try running powerd(8): > > echo 'powerd_enable=YES' >> /etc/rc.conf > > > > -- > > Nate > > I would like to run powerd, but that program wants to see the > .freq/.freq_levels sysctls at startup. > > I'm still trying determine why enabling ichss causes the machine's > temperature to spike from 57C to 90C. Weird. Try this patch perhaps. 1) It would be good to get it tested on a machine with ICHSS, and 2) there's a slim chance that a bug it fixes in the ICHSS setup may make a difference: --- //depot/vendor/freebsd/src/sys/amd64/amd64/legacy.c 2007/09/30 11:13:02 +++ //depot/user/jhb/acpipci/amd64/amd64/legacy.c 2008/03/04 18:34:01 @@ -132,20 +111,10 @@ legacy_attach(device_t dev) { device_t child; - int i; - - /* First, attach the CPU pseudo-driver. */ - for (i = 0; i <= mp_maxid; i++) - if (!CPU_ABSENT(i)) { - child = BUS_ADD_CHILD(dev, 0, "cpu", i); - if (child == NULL) - panic("legacy_attach cpu"); - device_probe_and_attach(child); - } /* - * Second, let our child driver's identify any child devices that - * they can find. Once that is done attach any devices that we + * Let our child drivers identify any child devices that they + * can find. Once that is done attach any devices that we * found. */ bus_generic_probe(dev); @@ -241,6 +210,7 @@ * Legacy CPU attachment when ACPI is not available. Drivers like * cpufreq(4) hang off this. */ +static void cpu_identify(driver_t *driver, device_t parent); static int cpu_read_ivar(device_t dev, device_t child, int index, uintptr_t *result); static device_t cpu_add_child(device_t bus, int order, const char *name, @@ -254,6 +224,7 @@ static device_method_t cpu_methods[] = { /* Device interface */ + DEVMETHOD(device_identify, cpu_identify), DEVMETHOD(device_probe, bus_generic_probe), DEVMETHOD(device_attach, bus_generic_attach), DEVMETHOD(device_detach, bus_generic_detach), @@ -287,6 +258,25 @@ static devclass_t cpu_devclass; DRIVER_MODULE(cpu, legacy, cpu_driver, cpu_devclass, 0, 0); +static void +cpu_identify(driver_t *driver, device_t parent) +{ + device_t child; + int i; + + /* + * Attach a cpuX device for each CPU. We use an order of 150 + * so that these devices are attached after the Host-PCI + * bridges (which are added at order 100). + */ + for (i = 0; i <= mp_maxid; i++) + if (!CPU_ABSENT(i)) { + child = BUS_ADD_CHILD(parent, 150, "cpu", i); + if (child == NULL) + panic("legacy_attach cpu"); + } +} + static device_t cpu_add_child(device_t bus, int order, const char *name, int unit) { --- //depot/vendor/freebsd/src/sys/dev/acpica/acpi.c 2008/01/28 02:00:16 +++ //depot/user/jhb/acpipci/dev/acpica/acpi.c 2008/02/28 13:51:06 @@ -1533,18 +1523,31 @@ static int acpi_probe_order(ACPI_HANDLE handle, int *order) { + ACPI_OBJECT_TYPE type; + u_int addr; /* * 1. I/O port and memory system resource holders * 2. Embedded controllers (to handle early accesses) * 3. PCI Link Devices + * 11 - 266. Host-PCI bridges sorted by _ADR + * 280. CPUs */ + AcpiGetType(handle, &type); if (acpi_MatchHid(handle, "PNP0C01") || acpi_MatchHid(handle, "PNP0C02")) *order = 1; else if (acpi_MatchHid(handle, "PNP0C09")) *order = 2; else if (acpi_MatchHid(handle, "PNP0C0F")) *order = 3; + else if (acpi_MatchHid(handle, "PNP0A03")) { + if (ACPI_SUCCESS(acpi_GetInteger(handle, "_ADR", &addr))) + *order = 11 + ACPI_ADR_PCI_SLOT(addr) * (PCI_FUNCMAX + 1) + + ACPI_ADR_PCI_FUNC(addr); + else + *order = 11; + } else if (type == ACPI_TYPE_PROCESSOR) + *order = 280; return (0); } @@ -1591,14 +1594,17 @@ break; /* - * Create a placeholder device for this node. Sort the placeholder - * so that the probe/attach passes will run breadth-first. Orders - * less than ACPI_DEV_BASE_ORDER are reserved for special objects - * (i.e., system resources). Larger values are used for all other - * devices. + * Create a placeholder device for this node. Sort the + * placeholder so that the probe/attach passes will run + * breadth-first. Orders less than ACPI_DEV_BASE_ORDER + * are reserved for special objects (i.e., system + * resources). Orders between ACPI_DEV_BASE_ORDER and 300 + * are used for Host-PCI bridges (and effectively all + * their children) and CPUs. Larger values are used for + * all other devices. */ ACPI_DEBUG_PRINT((ACPI_DB_OBJECTS, "scanning '%s'\n", handle_str)); - order = (level + 1) * ACPI_DEV_BASE_ORDER; + order = level * 10 + 300; acpi_probe_order(handle, &order); child = BUS_ADD_CHILD(bus, order, NULL, -1); if (child == NULL) --- //depot/vendor/freebsd/src/sys/dev/cpufreq/ichss.c 2006/05/16 14:41:44 +++ //depot/user/jhb/acpipci/dev/cpufreq/ichss.c 2008/03/04 18:27:52 @@ -91,7 +91,7 @@ (bus_space_write_1(rman_get_bustag((reg)), \ rman_get_bushandle((reg)), 0, (val))) -static int ichss_pci_probe(device_t dev); +static void ichss_identify(driver_t *driver, device_t parent); static int ichss_probe(device_t dev); static int ichss_attach(device_t dev); static int ichss_detach(device_t dev); @@ -103,6 +103,7 @@ static device_method_t ichss_methods[] = { /* Device interface */ + DEVMETHOD(device_identify, ichss_identify), DEVMETHOD(device_probe, ichss_probe), DEVMETHOD(device_attach, ichss_attach), DEVMETHOD(device_detach, ichss_detach), @@ -120,15 +121,7 @@ static devclass_t ichss_devclass; DRIVER_MODULE(ichss, cpu, ichss_driver, ichss_devclass, 0, 0); -static device_method_t ichss_pci_methods[] = { - DEVMETHOD(device_probe, ichss_pci_probe), - {0, 0} -}; -static driver_t ichss_pci_driver = { - "ichss_pci", ichss_pci_methods, 0 -}; -static devclass_t ichss_pci_devclass; -DRIVER_MODULE(ichss_pci, pci, ichss_pci_driver, ichss_pci_devclass, 0, 0); +static device_t ich_device; #if 0 #define DPRINT(x...) printf(x) @@ -136,70 +129,69 @@ #define DPRINT(x...) #endif -/* - * We detect the chipset by looking for its LPC bus ID during the PCI - * scan and reading its config registers during the probe. However, - * we add the ichss child under the cpu device since even though the - * chipset provides the control, it really affects the cpu only. - * - * XXX This approach does not work if the module is loaded after boot. - */ -static int -ichss_pci_probe(device_t dev) +static void +ichss_identify(driver_t *driver, device_t parent) { - device_t child, parent; + device_t child; uint32_t pmbase; + if (resource_disabled("ichss", 0)) + return; + /* - * TODO: add a quirk to disable if we see the 82815_MC along - * with the 82801BA and revision < 5. + * It appears that ICH SpeedStep only requires a single CPU to + * set the value (since the chipset is shared by all CPUs.) + * Thus, we only add a child to cpu 0. */ - if (pci_get_vendor(dev) != PCI_VENDOR_INTEL || - (pci_get_device(dev) != PCI_DEV_82801BA && - pci_get_device(dev) != PCI_DEV_82801CA && - pci_get_device(dev) != PCI_DEV_82801DB)) - return (ENXIO); + if (device_get_unit(parent) != 0) + return; - /* Only one CPU is supported for this hardware. */ - if (devclass_get_device(ichss_devclass, 0)) - return (ENXIO); + /* Avoid duplicates. */ + if (device_find_child(parent, "ichss", -1)) + return; /* - * Add a child under the CPU parent. It appears that ICH SpeedStep - * only requires a single CPU to set the value (since the chipset - * is shared by all CPUs.) Thus, we only add a child to cpu 0. + * ICH2/3/4-M I/O Controller Hub is at bus 0, slot 1F, function 0. + * E.g. see Section 6.1 "PCI Devices and Functions" and table 6.1 of + * Intel(r) 82801BA I/O Controller Hub 2 (ICH2) and Intel(r) 82801BAM + * I/O Controller Hub 2 Mobile (ICH2-M). + * + * TODO: add a quirk to disable if we see the 82815_MC along + * with the 82801BA and revision < 5. */ - parent = devclass_get_device(devclass_find("cpu"), 0); - KASSERT(parent != NULL, ("cpu parent is NULL")); - child = BUS_ADD_CHILD(parent, 0, "ichss", 0); - if (child == NULL) { - device_printf(parent, "add SpeedStep child failed\n"); - return (ENXIO); - } + ich_device = pci_find_bsf(0, 0x1f, 0); + if (ich_device == NULL || + pci_get_vendor(ich_device) != PCI_VENDOR_INTEL || + (pci_get_device(ich_device) != PCI_DEV_82801BA && + pci_get_device(ich_device) != PCI_DEV_82801CA && + pci_get_device(ich_device) != PCI_DEV_82801DB)) + return; /* Find the PMBASE register from our PCI config header. */ - pmbase = pci_read_config(dev, ICHSS_PMBASE_OFFSET, sizeof(pmbase)); + pmbase = pci_read_config(ich_device, ICHSS_PMBASE_OFFSET, + sizeof(pmbase)); if ((pmbase & ICHSS_IO_REG) == 0) { printf("ichss: invalid PMBASE memory type\n"); - return (ENXIO); + return; } pmbase &= ICHSS_PMBASE_MASK; if (pmbase == 0) { printf("ichss: invalid zero PMBASE address\n"); - return (ENXIO); + return; } DPRINT("ichss: PMBASE is %#x\n", pmbase); + child = BUS_ADD_CHILD(parent, 0, "ichss", 0); + if (child == NULL) { + device_printf(parent, "add SpeedStep child failed\n"); + return; + } + /* Add the bus master arbitration and control registers. */ bus_set_resource(child, SYS_RES_IOPORT, 0, pmbase + ICHSS_BM_OFFSET, 1); bus_set_resource(child, SYS_RES_IOPORT, 1, pmbase + ICHSS_CTRL_OFFSET, 1); - - /* Attach the new CPU child now. */ - device_probe_and_attach(child); - - return (ENXIO); } static int @@ -207,11 +199,7 @@ { device_t est_dev, perf_dev; int error, type; - uint16_t ss_en; - if (resource_disabled("ichss", 0)) - return (ENXIO); - /* * If the ACPI perf driver has attached and is not just offering * info, let it manage things. Also, if Enhanced SpeedStep is @@ -227,14 +215,6 @@ if (est_dev && device_is_attached(est_dev)) return (ENXIO); - /* Activate SpeedStep control if not already enabled. */ - ss_en = pci_read_config(dev, ICHSS_PMCFG_OFFSET, sizeof(ss_en)); - if ((ss_en & ICHSS_ENABLE) == 0) { - printf("ichss: enabling SpeedStep support\n"); - pci_write_config(dev, ICHSS_PMCFG_OFFSET, - ss_en | ICHSS_ENABLE, sizeof(ss_en)); - } - device_set_desc(dev, "SpeedStep ICH"); return (-1000); } @@ -243,6 +223,7 @@ ichss_attach(device_t dev) { struct ichss_softc *sc; + uint16_t ss_en; sc = device_get_softc(dev); sc->dev = dev; @@ -264,6 +245,14 @@ return (ENXIO); } + /* Activate SpeedStep control if not already enabled. */ + ss_en = pci_read_config(ich_device, ICHSS_PMCFG_OFFSET, sizeof(ss_en)); + if ((ss_en & ICHSS_ENABLE) == 0) { + device_printf(dev, "enabling SpeedStep support\n"); + pci_write_config(ich_device, ICHSS_PMCFG_OFFSET, + ss_en | ICHSS_ENABLE, sizeof(ss_en)); + } + /* Setup some defaults for our exported settings. */ sc->sets[0].freq = CPUFREQ_VAL_UNKNOWN; sc->sets[0].volts = CPUFREQ_VAL_UNKNOWN; --- //depot/vendor/freebsd/src/sys/i386/i386/legacy.c 2007/09/30 11:08:30 +++ //depot/user/jhb/acpipci/i386/i386/legacy.c 2008/03/04 18:34:01 @@ -137,20 +116,10 @@ legacy_attach(device_t dev) { device_t child; - int i; - - /* First, attach the CPU pseudo-driver. */ - for (i = 0; i <= mp_maxid; i++) - if (!CPU_ABSENT(i)) { - child = BUS_ADD_CHILD(dev, 0, "cpu", i); - if (child == NULL) - panic("legacy_attach cpu"); - device_probe_and_attach(child); - } /* - * Second, let our child driver's identify any child devices that - * they can find. Once that is done attach any devices that we + * Let our child drivers identify any child devices that they + * can find. Once that is done attach any devices that we * found. */ bus_generic_probe(dev); @@ -262,6 +231,7 @@ * Legacy CPU attachment when ACPI is not available. Drivers like * cpufreq(4) hang off this. */ +static void cpu_identify(driver_t *driver, device_t parent); static int cpu_read_ivar(device_t dev, device_t child, int index, uintptr_t *result); static device_t cpu_add_child(device_t bus, int order, const char *name, @@ -275,6 +245,7 @@ static device_method_t cpu_methods[] = { /* Device interface */ + DEVMETHOD(device_identify, cpu_identify), DEVMETHOD(device_probe, bus_generic_probe), DEVMETHOD(device_attach, bus_generic_attach), DEVMETHOD(device_detach, bus_generic_detach), @@ -308,6 +279,25 @@ static devclass_t cpu_devclass; DRIVER_MODULE(cpu, legacy, cpu_driver, cpu_devclass, 0, 0); +static void +cpu_identify(driver_t *driver, device_t parent) +{ + device_t child; + int i; + + /* + * Attach a cpuX device for each CPU. We use an order of 150 + * so that these devices are attached after the Host-PCI + * bridges (which are added at order 100). + */ + for (i = 0; i <= mp_maxid; i++) + if (!CPU_ABSENT(i)) { + child = BUS_ADD_CHILD(parent, 150, "cpu", i); + if (child == NULL) + panic("legacy_attach cpu"); + } +} + static device_t cpu_add_child(device_t bus, int order, const char *name, int unit) { -- John Baldwin