Page MenuHomeFreeBSD
Differential D55007 Diff 171792 sys/x86/cpufreq/hwpstate_amd.c

Changeset View

Standalone View

View Options

sys/x86/cpufreq/hwpstate_amd.c

Show First 20 Lines • Show All 655 Lines • ▼ Show 20 Linesstruct set_autonomous_hwp_data {
uint64_t request; uint64_t request;
}; };
static void static void
amd_set_autonomous_hwp_cb(void *args) amd_set_autonomous_hwp_cb(void *args)
{ {
struct set_autonomous_hwp_data *const data = args; struct set_autonomous_hwp_data *const data = args;
struct hwpstate_softc *const sc = data->sc; struct hwpstate_softc *const sc = data->sc;
uint64_t lowest_perf, highest_perf;
int error; int error;
/* We proceed sequentially, so we'll clear out errors on progress. */ /* We proceed sequentially, so we'll clear out errors on progress. */
data->res = HWP_ERROR_CPPC_ENABLE | HWP_ERROR_CPPC_CAPS | data->res = HWP_ERROR_CPPC_ENABLE | HWP_ERROR_CPPC_CAPS |
HWP_ERROR_CPPC_REQUEST | HWP_ERROR_CPPC_REQUEST_WRITE; HWP_ERROR_CPPC_REQUEST | HWP_ERROR_CPPC_REQUEST_WRITE;
error = wrmsr_safe(MSR_AMD_CPPC_ENABLE, 1); error = wrmsr_safe(MSR_AMD_CPPC_ENABLE, 1);
if (error != 0) if (error != 0)
Show All 13 Linesamd_set_autonomous_hwp_cb(void *args)
data->init_request = sc->cppc.request; data->init_request = sc->cppc.request;
/* /*
* In Intel's reference manual, the default value of EPP is 0x80u which * In Intel's reference manual, the default value of EPP is 0x80u which
* is the balanced mode. For consistency, we set the same value in AMD's * is the balanced mode. For consistency, we set the same value in AMD's
* CPPC driver. * CPPC driver.
*/ */
SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_EPP_BITS, 0x80); SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_EPP_BITS, 0x80);
/* Enable autonomous mode by setting desired performance to 0. */
SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_DES_PERF_BITS, 0);
/*
* When MSR_AMD_CPPC_CAPS_1 stays at its reset value (0) before CPPC
* activation (not supposed to happen, but happens in the field), we use
* reasonable default values that are explicitly described by the ACPI
* spec (all 0s for the minimum value, all 1s for the maximum one).
* Going further, we actually do the same as long as the minimum and
* maximum performance levels are not sorted or are equal (in which case
* CPPC is not supposed to make sense at all), which covers the reset
* value case.
*/
lowest_perf = BITS_VALUE(AMD_CPPC_CAPS_1_LOWEST_PERF_BITS, data->caps);
highest_perf = BITS_VALUE(AMD_CPPC_CAPS_1_HIGHEST_PERF_BITS, data->caps);
if (lowest_perf >= highest_perf) {
lowest_perf = 0;
highest_perf = -1;
}
SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_MIN_PERF_BITS, SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_MIN_PERF_BITS,
BITS_VALUE(AMD_CPPC_CAPS_1_LOWEST_PERF_BITS, data->caps)); lowest_perf);
SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_MAX_PERF_BITS, SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_MAX_PERF_BITS,
BITS_VALUE(AMD_CPPC_CAPS_1_HIGHEST_PERF_BITS, data->caps)); highest_perf);
/* enable autonomous mode by setting desired performance to 0 */
SET_BITS_VALUE(sc->cppc.request, AMD_CPPC_REQUEST_DES_PERF_BITS, 0);
error = wrmsr_safe(MSR_AMD_CPPC_REQUEST, sc->cppc.request); error = wrmsr_safe(MSR_AMD_CPPC_REQUEST, sc->cppc.request);
if (error != 0) if (error != 0)
return; return;
data->res &= ~HWP_ERROR_CPPC_REQUEST_WRITE; data->res &= ~HWP_ERROR_CPPC_REQUEST_WRITE;
data->request = sc->cppc.request; data->request = sc->cppc.request;
} }
static int static int
amd_set_autonomous_hwp(struct hwpstate_softc *sc) amd_set_autonomous_hwp(struct hwpstate_softc *sc)
{ {
const device_t dev = sc->dev; const device_t dev = sc->dev;
const u_int cpuid = cpu_get_pcpu(dev)->pc_cpuid; const u_int cpuid = cpu_get_pcpu(dev)->pc_cpuid;
struct set_autonomous_hwp_data data; struct set_autonomous_hwp_data data;
aokblastUnsubmitted
Done
Inline Actions

Is there any reference saying that a PC manufacture cannot configure the _CPC object in their DSDT to have lowest == hightest if they only want to provide one performace level?

ACPI spec sayws that

Not all performance levels need be unique. A platform’s nominal performance level may also be its highest performance level, for example.*

aokblast: Is there any reference saying that a PC manufacture cannot configure the _CPC object in their…
olceAuthorUnsubmitted
Done
Inline Actions

I don't remember seeing such an explicit sentence, but to me it's just common sense, simply because there's no point in supporting CPPC at all if there is a single performance level.

That quote from ACPI says that some reference levels can be the same, but it typically mentions the "nominal" performance one and the "highest" one. If the minimum and maximum ones are the same, then every other is necessarily the same, and turning on CPPC simply has no effect. In this case, the minimum and maximum values we set cannot have any influence anyway.

Intel docs say that out of range values for performance in the request register are just clipped to reasonable ones and silently (if you read the values back, you'll get what you set, even if it's not what the hardware actually uses). Although AMD's doc is not explicit, I'm pretty sure they are doing exactly the same (faulting the processor in this case would really be silly; at the very least, an exception could be triggered, but that is not mentioned by the document and to the best of my knowledge we have not seen any hint that such behavior can occur; I've taken some time to improve error handling to be very precise, so if something like this materializes, we'll know, and we'll continue running).

From the user's point of view, it would be quite confusing that we initialize with not-correctly-sorted minimum and maximum values, even if the hardware does so in cracauer@'s case (directly in the request register; the capability 1 register is entirely zero).

Going forward, I'm considering having default settings of CPPC request the maximum performance, to avoid regressions as CPPC is enabled by default and cpufreq is included in GENERIC. See PR 292615, and please comment there about that if you have some thoughts.

olce: I don't remember seeing such an explicit sentence, but to me it's just common sense, simply…
aokblastUnsubmitted
Done
Inline Actions

The statement comes from here under 8.4.6.1.1. Performance Capabilities / Thresholds.

Sorry that I cannot fully follow up the bugzilla since I am busy. I know the problem in that PR so my suggestion is something like

lowest_perf > highest_perf || highest_perf == 0

But you are right lowest_perf == highest_perf makes no sense although the spec says it is legal . You can just let this patch go ahead.

aokblast: The statement comes from [[ https://uefi.org/specs/ACPI/6.
olceAuthorUnsubmitted
Done
Inline Actions

Sorry that I cannot fully follow up the bugzilla since I am busy.

Sure, no problem. I just wanted to say that if you have ideas on that matter, it's best to comment there than in this revision which is about a different topic.

olce: > Sorry that I cannot fully follow up the bugzilla since I am busy. Sure, no problem. I just…
struct sbuf sbs; struct sbuf sbs;
struct sbuf *sb; struct sbuf *sb;
data.sc = sc; data.sc = sc;
smp_rendezvous_cpu(cpuid, smp_no_rendezvous_barrier, smp_rendezvous_cpu(cpuid, smp_no_rendezvous_barrier,
amd_set_autonomous_hwp_cb, smp_no_rendezvous_barrier, &data); amd_set_autonomous_hwp_cb, smp_no_rendezvous_barrier, &data);
if (hwp_has_error(data.res, HWP_ERROR_CPPC_ENABLE)) { if (hwp_has_error(data.res, HWP_ERROR_CPPC_ENABLE)) {
▲ Show 20 Lines • Show All 312 Lines • Show Last 20 Lines