From owner-freebsd-acpi@FreeBSD.ORG Fri Feb 22 16:02:30 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 E689F16A400; Fri, 22 Feb 2008 16:02:30 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from falcon.cybervisiontech.com (falcon.cybervisiontech.com [217.20.163.9]) by mx1.freebsd.org (Postfix) with ESMTP id 6C01B13C447; Fri, 22 Feb 2008 16:02:30 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from localhost (localhost [127.0.0.1]) by falcon.cybervisiontech.com (Postfix) with ESMTP id 819E243F99A; Fri, 22 Feb 2008 18:02:29 +0200 (EET) X-Virus-Scanned: Debian amavisd-new at falcon.cybervisiontech.com Received: from falcon.cybervisiontech.com ([127.0.0.1]) by localhost (falcon.cybervisiontech.com [127.0.0.1]) (amavisd-new, port 10027) with ESMTP id pG2Bteft80vS; Fri, 22 Feb 2008 18:02:29 +0200 (EET) Received: from [10.74.70.239] (unknown [193.138.145.53]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by falcon.cybervisiontech.com (Postfix) with ESMTP id 0959E43F783; Fri, 22 Feb 2008 18:02:27 +0200 (EET) Message-ID: <47BEF211.4030608@icyb.net.ua> Date: Fri, 22 Feb 2008 18:02:25 +0200 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.9 (X11/20071208) MIME-Version: 1.0 To: Nate Lawson References: <47B96989.6070008@icyb.net.ua> <98F7E48C-1CBF-494D-8411-D80E25247214@FreeBSD.org> <47BAF97A.80405@icyb.net.ua> <47BAFB27.1050509@root.org> <47BB2D1F.8000008@icyb.net.ua> <47BEF0C1.5090800@icyb.net.ua> In-Reply-To: <47BEF0C1.5090800@icyb.net.ua> Content-Type: multipart/mixed; boundary="------------040300070806020408060602" Cc: freebsd-acpi@freebsd.org Subject: Re: acpi_throttle: quirk based on pci info 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, 22 Feb 2008 16:02:31 -0000 This is a multi-part message in MIME format. --------------040300070806020408060602 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit on 22/02/2008 17:56 Andriy Gapon said the following: > Please find attached a patch that makes sure that acpi_thermal is > initialized after PCI bus is available, so that it is possible to decide > about hardware quirks based on PCI information. > The code uses the same approach as ichss does. > The patch is tested to work. Oops! I attached on older version of the patch, sorry. Correct patch is here. (parent of acpi_throttle is cpu, not acpi) > NOTE: This patch in contaminated! It contains code that forces throttle > duty width to be 3 bits for PIIX4E and PIIX4M. This is in accordance > with the chipsets specifications. Some motherboard/bios vendors lie > about this value in FADT. > If not accepted, this quirk can be easily stripped from the patch as its > code is contained in distinct hunks. -- Andriy Gapon --------------040300070806020408060602 Content-Type: text/x-patch; name="acpi_throttle-piix4-width.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="acpi_throttle-piix4-width.patch" --- acpi_throttle.c.orig 2008-02-21 21:08:28.000000000 +0200 +++ acpi_throttle.c 2008-02-21 23:55:25.000000000 +0200 @@ -80,18 +80,21 @@ (CPU_SPEED_PERCENT(x) % 10) #define CPU_P_CNT_THT_EN (1<<4) #define CPU_QUIRK_NO_THROTTLE (1<<1) /* Throttling is not usable. */ +#define CPU_QUIRK_PIIX4_WIDTH (1<<2) #define PCI_VENDOR_INTEL 0x8086 #define PCI_DEVICE_82371AB_3 0x7113 /* PIIX4 chipset for quirks. */ #define PCI_REVISION_A_STEP 0 #define PCI_REVISION_B_STEP 1 +#define PCI_REVISION_4E 2 +#define PCI_REVISION_4M 3 static uint32_t cpu_duty_offset; /* Offset in P_CNT of throttle val. */ static uint32_t cpu_duty_width; /* Bit width of throttle value. */ static int thr_rid; /* Driver-wide resource id. */ static int thr_quirks; /* Indicate any hardware bugs. */ -static void acpi_throttle_identify(driver_t *driver, device_t parent); +static int acpi_throttle_pci_probe(device_t dev); static int acpi_throttle_probe(device_t dev); static int acpi_throttle_attach(device_t dev); static int acpi_throttle_evaluate(struct acpi_throttle_softc *sc); @@ -104,7 +107,6 @@ static device_method_t acpi_throttle_methods[] = { /* Device interface */ - DEVMETHOD(device_identify, acpi_throttle_identify), DEVMETHOD(device_probe, acpi_throttle_probe), DEVMETHOD(device_attach, acpi_throttle_attach), @@ -125,25 +127,46 @@ static devclass_t acpi_throttle_devclass; DRIVER_MODULE(acpi_throttle, cpu, acpi_throttle_driver, acpi_throttle_devclass, 0, 0); +MODULE_DEPEND(acpi_throttle, acpi, 1, 1, 1); -static void -acpi_throttle_identify(driver_t *driver, device_t parent) +static device_method_t acpi_throttle_pci_methods[] = { + DEVMETHOD(device_probe, acpi_throttle_pci_probe), + {0, 0} +}; + +static driver_t acpi_throttle_pci_driver = { + "acpi_throttle_pci", + acpi_throttle_pci_methods, + 0 +}; + +static devclass_t acpi_throttle_pci_devclass; +DRIVER_MODULE(acpi_throttle_pci, pci, acpi_throttle_pci_driver, + acpi_throttle_pci_devclass, 0, 0); + +static int +acpi_throttle_pci_probe(device_t dev) { ACPI_BUFFER buf; ACPI_HANDLE handle; ACPI_OBJECT *obj; + device_t parent; + device_t child; + + parent = devclass_get_device(devclass_find("cpu"), 0); + KASSERT(parent != NULL, ("cpu parent is NULL")); /* Make sure we're not being doubly invoked. */ if (device_find_child(parent, "acpi_throttle", -1)) - return; + return (ENXIO); /* Check for a valid duty width and parent CPU type. */ handle = acpi_get_handle(parent); if (handle == NULL) - return; + return (ENXIO); if (AcpiGbl_FADT.DutyWidth == 0 || acpi_get_type(parent) != ACPI_TYPE_PROCESSOR) - return; + return (ENXIO); /* * Add a child if there's a non-NULL P_BLK and correct length, or @@ -152,14 +175,18 @@ buf.Pointer = NULL; buf.Length = ACPI_ALLOCATE_BUFFER; if (ACPI_FAILURE(AcpiEvaluateObject(handle, NULL, NULL, &buf))) - return; + return (ENXIO); obj = (ACPI_OBJECT *)buf.Pointer; if ((obj->Processor.PblkAddress && obj->Processor.PblkLength >= 4) || ACPI_SUCCESS(AcpiEvaluateObject(handle, "_PTC", NULL, NULL))) { - if (BUS_ADD_CHILD(parent, 0, "acpi_throttle", -1) == NULL) + child = BUS_ADD_CHILD(parent, 0, "acpi_throttle", -1); + if (child == NULL) device_printf(parent, "add throttle child failed\n"); + else + device_probe_and_attach(child); } AcpiOsFree(obj); + return (ENXIO); } static int @@ -247,6 +274,8 @@ } if (cpu_duty_width == 0 || (thr_quirks & CPU_QUIRK_NO_THROTTLE) != 0) return (ENXIO); + if ((thr_quirks & CPU_QUIRK_PIIX4_WIDTH) != 0) + cpu_duty_width = 3; /* Validate the duty offset/width. */ duty_end = cpu_duty_offset + cpu_duty_width - 1; @@ -333,8 +362,14 @@ case PCI_REVISION_A_STEP: case PCI_REVISION_B_STEP: thr_quirks |= CPU_QUIRK_NO_THROTTLE; + device_printf(sc->cpu_dev, + "throttling is disabled for PIIX4 rev. A/B\n"); break; - default: + case PCI_REVISION_4E: + case PCI_REVISION_4M: + thr_quirks |= CPU_QUIRK_PIIX4_WIDTH; + device_printf(sc->cpu_dev, + "throttling has 12.5%% step for PIIX4E/M\n"); break; } } --------------040300070806020408060602--