Page MenuHomeFreeBSD

hwpstate_amd(4): Sane defaults for min/max perf on insane capabilities
AcceptedPublic

Authored by olce on Sat, Jan 31, 11:48 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Feb 3, 11:02 PM
Unknown Object (File)
Sun, Feb 1, 11:51 PM
Unknown Object (File)
Sun, Feb 1, 9:58 AM
Unknown Object (File)
Sun, Feb 1, 2:10 AM
Unknown Object (File)
Sun, Feb 1, 12:31 AM
Unknown Object (File)
Sat, Jan 31, 11:58 PM
Subscribers

Details

Summary

If the CPPC_CAPABILITY_1 register stays at its reset value (0) even
after enabling CPPC, as observed in the field (see the referenced PR
below), use sane min/max performance limits as hinted by the ACPI spec,
i.e., all 0s for the minimum value and all 1s for the maximum one.

While here, let's cope upfront with some more insane situations, where
the minimum value would be greater than the maximum one, but also if
they would be equal which does not seem to make sense at all in the CPPC
frame. That last case actually also covers the one exposed in the
previous paragraph.

PR: 292615
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 70279
Build 67162: arc lint + arc unit

Event Timeline

olce requested review of this revision.Sat, Jan 31, 11:48 AM
sys/x86/cpufreq/hwpstate_amd.c
719

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.*

olce marked an inline comment as done.Sat, Feb 7, 10:00 AM
olce added inline comments.
sys/x86/cpufreq/hwpstate_amd.c
719

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.

aokblast added inline comments.
sys/x86/cpufreq/hwpstate_amd.c
719

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.

This revision is now accepted and ready to land.Sun, Feb 8, 1:49 AM
olce marked 2 inline comments as done.Tue, Feb 10, 4:15 PM
olce added inline comments.
sys/x86/cpufreq/hwpstate_amd.c
719

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 marked an inline comment as done.Tue, Feb 10, 4:15 PM