Changeset View
Standalone View
sys/x86/cpufreq/est.c
Show First 20 Lines • Show All 44 Lines • ▼ Show 20 Lines | |||||
#include <machine/md_var.h> | #include <machine/md_var.h> | ||||
#include <machine/specialreg.h> | #include <machine/specialreg.h> | ||||
#include <contrib/dev/acpica/include/acpi.h> | #include <contrib/dev/acpica/include/acpi.h> | ||||
#include <dev/acpica/acpivar.h> | #include <dev/acpica/acpivar.h> | ||||
#include "acpi_if.h" | #include "acpi_if.h" | ||||
/* Status/control registers (from the IA-32 System Programming Guide). */ | /* Status/control registers (from the IA-32 System Programming Guide). */ | ||||
cem: Would it be possible to define this in a shared header instead of multiple `extern` references… | |||||
Done Inline Actionsyeah... jhb@ recommended this way. I'm fine with whatever though, recommendations? bwidawsk: yeah... jhb@ recommended this way. I'm fine with whatever though, recommendations? | |||||
Not Done Inline ActionsLeave it be for now, I guess. This variable is a gross hack, hopefully it isn't used in additional places. cem: Leave it be for now, I guess. This variable is a gross hack, hopefully it isn't used in… | |||||
Not Done Inline ActionsI mostly can't think of a non-crappy header to put it in. jhb: I mostly can't think of a non-crappy header to put it in. | |||||
#define MSR_PERF_STATUS 0x198 | #define MSR_PERF_STATUS 0x198 | ||||
#define MSR_PERF_CTL 0x199 | #define MSR_PERF_CTL 0x199 | ||||
/* Register and bit for enabling SpeedStep. */ | /* Register and bit for enabling SpeedStep. */ | ||||
#define MSR_MISC_ENABLE 0x1a0 | #define MSR_MISC_ENABLE 0x1a0 | ||||
#define MSR_SS_ENABLE (1<<16) | #define MSR_SS_ENABLE (1<<16) | ||||
/* Frequency and MSR control values. */ | /* Frequency and MSR control values. */ | ||||
▲ Show 20 Lines • Show All 784 Lines • ▼ Show 20 Lines | static cpu_info ESTprocs[] = { | ||||
CENTAUR(C7M_775_ULV, 1500, 956, 400, 796, 100), | CENTAUR(C7M_775_ULV, 1500, 956, 400, 796, 100), | ||||
CENTAUR(C7M_771, 1200, 860, 400, 844, 100), | CENTAUR(C7M_771, 1200, 860, 400, 844, 100), | ||||
CENTAUR(C7M_772_ULV, 1200, 844, 400, 796, 100), | CENTAUR(C7M_772_ULV, 1200, 844, 400, 796, 100), | ||||
CENTAUR(C7M_779_ULV, 1000, 796, 400, 796, 100), | CENTAUR(C7M_779_ULV, 1000, 796, 400, 796, 100), | ||||
CENTAUR(C7M_770_ULV, 1000, 844, 400, 796, 100), | CENTAUR(C7M_770_ULV, 1000, 844, 400, 796, 100), | ||||
{ 0, 0, NULL }, | { 0, 0, NULL }, | ||||
}; | }; | ||||
void intel_hwpstate_identify(driver_t *driver, device_t parent); | |||||
cemUnsubmitted Done Inline ActionsProbably put this in a header (it can be a totally trivial intel_cpufreq_internal.h, for example) rather than declaring it in both drivers. cem: Probably put this in a header (it can be a totally trivial `intel_cpufreq_internal.h`, for… | |||||
static void est_identify(driver_t *driver, device_t parent); | static void est_identify(driver_t *driver, device_t parent); | ||||
static int est_features(driver_t *driver, u_int *features); | static int est_features(driver_t *driver, u_int *features); | ||||
static int est_probe(device_t parent); | static int est_probe(device_t parent); | ||||
static int est_attach(device_t parent); | static int est_attach(device_t parent); | ||||
static int est_detach(device_t parent); | static int est_detach(device_t parent); | ||||
static int est_get_info(device_t dev); | static int est_get_info(device_t dev); | ||||
static int est_acpi_info(device_t dev, freq_info **freqs, | static int est_acpi_info(device_t dev, freq_info **freqs, | ||||
size_t *freqslen); | size_t *freqslen); | ||||
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines | est_features(driver_t *driver, u_int *features) | ||||
*features = ACPI_CAP_PERF_MSRS | ACPI_CAP_C1_IO_HALT; | *features = ACPI_CAP_PERF_MSRS | ACPI_CAP_C1_IO_HALT; | ||||
return (0); | return (0); | ||||
} | } | ||||
static void | static void | ||||
est_identify(driver_t *driver, device_t parent) | est_identify(driver_t *driver, device_t parent) | ||||
{ | { | ||||
device_t child; | device_t child; | ||||
/* | |||||
* Defer to hwpstate if it is present. This priority logic | |||||
Done Inline ActionsOh, I see. This is the hack that is preventing multiple freq drivers from registering on intel. Doesn't this break est on hardware too old for HWP support? And the kludge only works for Intel devices? cem: Oh, I see. This is the hack that is preventing multiple freq drivers from registering on intel. | |||||
Done Inline ActionsIt shouldn't. In the hwpstate_intel code, if the hardware is too old, it sets the bool to false. Same for basically any unsupported hardware. bwidawsk: It shouldn't. In the hwpstate_intel code, if the hardware is too old, it sets the bool to false. | |||||
Done Inline ActionsSure, but what controls the order in which est_identify / intel_hwpstate_identify run such that the latter is guaranteed to run first? cem: Sure, but what controls the order in which est_identify / intel_hwpstate_identify run such that… | |||||
Done Inline ActionsI have wondered if we don't want to effectively merge the two drivers in some way. jhb: I have wondered if we don't want to effectively merge the two drivers in some way. | |||||
Done Inline ActionsI can't think of an immediate downside to this idea (and like it better than the current approach). Ben, is it objectionable to you? cem: I can't think of an immediate downside to this idea (and like it better than the current… | |||||
Done Inline ActionsThe main downside is that suppose after this is in I do the cpufreq rework I sketched out in my overall comment last week. In that world, it's quite easy to have the 'speedshift' driver use a higher probe priority than est and have them be separate drivers. Given that they work quite differently I think this is cleaner long term. I had typed this comment above before my general comment (so it was "earlier" in my thinking). jhb: The main downside is that suppose after this is in I do the cpufreq rework I sketched out in my… | |||||
Done Inline ActionsWhen and if you get a chance to do that rework (I don't think it's happened yet?), the drivers could be pulled apart again. It's one of those "the tree isn't written in stone, we can always change it later" plus "don't block working things on good ideas no one has time for" deals, IMO. cem: When and if you get a chance to do that rework (I don't think it's happened yet?), the drivers… | |||||
Done Inline ActionsDoes this seem like a reasonable way to intertwine the two identify routines? scottph: Does this seem like a reasonable way to intertwine the two identify routines? | |||||
Not Done Inline ActionsIt's kinda funky, but tolerable IMO. If John has strong objections, he overrules me :-). Newbus doesn't really make this easy/clean on us. cem: It's kinda funky, but tolerable IMO. If John has strong objections, he overrules me :-). | |||||
Not Done Inline ActionsThis is fine for now. jhb: This is fine for now. | |||||
* should be replaced with normal newbus probing in the | |||||
* future. | |||||
*/ | |||||
intel_hwpstate_identify(NULL, parent); | |||||
if (device_find_child(parent, "hwpstate_intel", -1) != NULL) | |||||
return; | |||||
/* Make sure we're not being doubly invoked. */ | /* Make sure we're not being doubly invoked. */ | ||||
if (device_find_child(parent, "est", -1) != NULL) | if (device_find_child(parent, "est", -1) != NULL) | ||||
return; | return; | ||||
/* Check that CPUID is supported and the vendor is Intel.*/ | /* Check that CPUID is supported and the vendor is Intel.*/ | ||||
if (cpu_high == 0 || (cpu_vendor_id != CPU_VENDOR_INTEL && | if (cpu_high == 0 || (cpu_vendor_id != CPU_VENDOR_INTEL && | ||||
cpu_vendor_id != CPU_VENDOR_CENTAUR)) | cpu_vendor_id != CPU_VENDOR_CENTAUR)) | ||||
▲ Show 20 Lines • Show All 432 Lines • Show Last 20 Lines |
Would it be possible to define this in a shared header instead of multiple extern references in .c files?