acpi_throttle: quirk based on pci info

[Rui Paulo]

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