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" | ||||
extern uint32_t intel_speed_shift; | |||||
cem: Would it be possible to define this in a shared header instead of multiple `extern` references… | |||||
bwidawskUnsubmitted 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? | |||||
cemUnsubmitted 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… | |||||
jhbUnsubmitted 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. | |||||
/* Status/control registers (from the IA-32 System Programming Guide). */ | /* Status/control registers (from the IA-32 System Programming Guide). */ | ||||
#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) | ||||
▲ Show 20 Lines • Show All 785 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 }, | ||||
}; | }; | ||||
static void est_identify(driver_t *driver, device_t parent); | static void est_identify(driver_t *driver, device_t parent); | ||||
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 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); | ||||
static int est_table_info(device_t dev, uint64_t msr, freq_info **freqs, | static int est_table_info(device_t dev, uint64_t msr, freq_info **freqs, | ||||
▲ Show 20 Lines • Show All 47 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; | ||||
/* If the Intel driver is handling this */ | |||||
if (intel_speed_shift == true) | |||||
cemUnsubmitted 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. | |||||
bwidawskUnsubmitted 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. | |||||
cemUnsubmitted 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… | |||||
jhbUnsubmitted 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. | |||||
cemUnsubmitted 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… | |||||
jhbUnsubmitted 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… | |||||
cemUnsubmitted 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… | |||||
scottphAuthorUnsubmitted 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? | |||||
cemUnsubmitted 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 :-). | |||||
jhbUnsubmitted Not Done Inline ActionsThis is fine for now. jhb: This is fine for now. | |||||
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?