Page MenuHomeFreeBSD

hwpstate_amd: Refactor by brancless version
Needs ReviewPublic

Authored by aokblast on Sun, Mar 1, 2:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 8, 10:40 AM
Unknown Object (File)
Sat, Mar 7, 9:09 PM
Unknown Object (File)
Fri, Mar 6, 9:00 PM
Unknown Object (File)
Wed, Mar 4, 5:40 PM
Unknown Object (File)
Wed, Mar 4, 5:07 PM
Unknown Object (File)
Mon, Mar 2, 6:09 PM
Unknown Object (File)
Mon, Mar 2, 4:49 PM
Unknown Object (File)
Mon, Mar 2, 5:42 AM
Subscribers

Details

Reviewers
olce

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 71169
Build 68052: arc lint + arc unit

Event Timeline

Idea is great. Currently, however, this revision depends on D55477 (which is rejected), so it needs to be re-based to become independent.

sys/x86/cpufreq/hwpstate_amd.c
642–645

I'd rather keep the existing style here.

aokblast added inline comments.
sys/x86/cpufreq/hwpstate_amd.c
642–645

It is removed so I mark it as done.

olce requested changes to this revision.Mon, Mar 9, 8:59 AM

Some small changes to do. Main point is to preserve sysctl_cppc_dump_handler()'s goal which is to always query the hardware (and thus never rely on cached values).

sys/x86/cpufreq/hwpstate_amd.c
179–180

CAPABILITY_1 is 64-bit (even if the upper 32-bits are reserved for the time being).

This change should be done in another revision/commit. What is your goal here? IMO, it would be useful to create sysctl knobs exporting the capability bounds; we'll have to apply the same kind of workaround for an uninitialized/improperly initialized register as we are currently applying for min/max performance value (in enable_cppc_cb()).

297–298

We really want sysctl_cppc_dump_handler() to always query CPUs and not use cache values (useful for debugging), which this change is in contradiction with, so please remove it, along with the noted related ones below.

316–319

Same as above.

358

Same as above.

873–874

As the addition of caps1 in the softc, belongs to a separate commit. See the corresponding inline comment for more details.

This revision now requires changes to proceed.Mon, Mar 9, 8:59 AM
aokblast marked an inline comment as done.

Minor fixes