From owner-freebsd-acpi@FreeBSD.ORG Fri Feb 22 17:52:59 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 4316B16A402 for ; Fri, 22 Feb 2008 17:52:59 +0000 (UTC) (envelope-from rpaulo@gmail.com) Received: from fk-out-0910.google.com (fk-out-0910.google.com [209.85.128.187]) by mx1.freebsd.org (Postfix) with ESMTP id C127313C448 for ; Fri, 22 Feb 2008 17:52:58 +0000 (UTC) (envelope-from rpaulo@gmail.com) Received: by fk-out-0910.google.com with SMTP id b27so619649fka.11 for ; Fri, 22 Feb 2008 09:52:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:cc:message-id:from:to:in-reply-to:content-type:content-transfer-encoding:mime-version:subject:date:references:x-mailer:sender; bh=erlxXRcRv3Jqyqqvm6mXAqL0P9rM4aGiF4jQQpruAak=; b=XQ3gxd+SY8IhUM/buDxhtcUN7PA74aOfq7UgKDy7vDkSoDxPYdEz7KN0KwPjimmVh7OFolME5f1z2FZL0TNFF9hN1a5Q3u4ev+E3fzkHNfej4VrTBybpyZhXcJ8UNfL/xIT2013PpOd512NBgWoNDjpIxuSFEYiITfanexxnhp0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=cc:message-id:from:to:in-reply-to:content-type:content-transfer-encoding:mime-version:subject:date:references:x-mailer:sender; b=jjHAhtkogDy1yt9J8HOlmPHiIo0XEAHUIfvT9dTaRjVByCL+612pQvemc9cXtX3qZopnWmOoO4QItYNU0gMGCQd9YaTvt9MyBtcv0fAgEj02EJw3MwniG9L/oKo79AiIjxPPT7KFgeS7wi3X0JEaLbUhLksXhFSvsNXzF2jZOr4= Received: by 10.82.113.6 with SMTP id l6mr465809buc.22.1203702776564; Fri, 22 Feb 2008 09:52:56 -0800 (PST) Received: from ?88.214.149.122? ( [88.214.149.122]) by mx.google.com with ESMTPS id p10sm1587572gvf.8.2008.02.22.09.52.53 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 22 Feb 2008 09:52:55 -0800 (PST) Message-Id: <96814C17-2A19-4EA5-89B6-9B9DA55252ED@freebsd.org> From: Rui Paulo To: Andriy Gapon In-Reply-To: <47BEF211.4030608@icyb.net.ua> Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit Mime-Version: 1.0 (Apple Message framework v919.2) Date: Fri, 22 Feb 2008 17:52:50 +0000 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> <47BEF211.4030608@icyb.net.ua> X-Mailer: Apple Mail (2.919.2) Sender: Rui Paulo 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 17:52:59 -0000 I'll handle this. Thanks! On Feb 22, 2008, at 4:02 PM, Andriy Gapon wrote: > 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 > --- 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; > } > } -- Rui Paulo