Date: Fri, 22 Feb 2008 17:52:50 +0000 From: Rui Paulo <rpaulo@freebsd.org> To: Andriy Gapon <avg@icyb.net.ua> Cc: freebsd-acpi@freebsd.org Subject: Re: acpi_throttle: quirk based on pci info Message-ID: <96814C17-2A19-4EA5-89B6-9B9DA55252ED@freebsd.org> In-Reply-To: <47BEF211.4030608@icyb.net.ua> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?96814C17-2A19-4EA5-89B6-9B9DA55252ED>