Page MenuHomeFreeBSD
Differential D47985

sys/x86/cpufreq/hwpstate_amd.c: add amd pstate for zen5
AcceptedPublic
Actions

Authored by aokblast on Sun, Dec 8, 8:55 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 15, 1:02 AM
Unknown Object (File)
Thu, Dec 12, 1:09 AM
Subscribers
emaste
git_danielschaefer.me
imp

Details

Reviewers
lwhsu
markj
jhb
cem

Diff Detail

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

Event Timeline

aokblast created this revision.Sun, Dec 8, 8:55 AM
Herald added a subscriber: imp. · View Herald TranscriptSun, Dec 8, 8:55 AM
aokblast requested review of this revision.Sun, Dec 8, 8:55 AM
Harbormaster completed remote builds in B61030: Diff 147692.Sun, Dec 8, 8:55 AM
aokblast added a reviewer: lwhsu.Sun, Dec 8, 9:00 AM
lwhsu added reviewers: markj, jhb, cem.Sun, Dec 8, 5:41 PM
cem added a comment.Sun, Dec 8, 5:59 PM

Is the associated hardware documentation public, and if so, could you link to it?

sys/x86/cpufreq/hwpstate_amd.c
533–534

Missing break

cem added inline comments.Sun, Dec 8, 6:02 PM
sys/x86/cpufreq/hwpstate_amd.c
95–97

I think we can just keep using the 17H macros for these three fields that didn't change? (IDIV, IDD, VID)

aokblast updated this revision to Diff 147714.Mon, Dec 9, 3:19 AM

Fix break error and add reference

Harbormaster completed remote builds in B61041: Diff 147714.Mon, Dec 9, 3:19 AM
markj added a comment.Mon, Dec 9, 2:26 PM
In D47985#1093902, @cem wrote:

Is the associated hardware documentation public, and if so, could you link to it?

I found it in volume 1 here: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/57896.zip

sys/x86/cpufreq/hwpstate_amd.c
93

This comment is repeated below, so isn't really needed here.

171

As a matter of style, it'd be a bit nicer to return the value instead of writing through a pointer.

489
491

According to the PPR, fid <= 0xf is a reserved value. Should we report an error in that case, instead of silently leaving the frequency unset?

aokblast updated this revision to Diff 147728.Mon, Dec 9, 2:44 PM

Fix style and report error for fid

Harbormaster completed remote builds in B61045: Diff 147728.Mon, Dec 9, 2:44 PM
aokblast updated this revision to Diff 147730.Mon, Dec 9, 2:47 PM

Update debug message for fid

Harbormaster completed remote builds in B61046: Diff 147730.Mon, Dec 9, 2:47 PM
aokblast added a comment.Mon, Dec 9, 2:49 PM

BTW, is hwpstate_amd quite older than intel one? I thought it don't have autonomous mode and it throttling all cpu into same freqs.

markj added a comment.Mon, Dec 9, 3:10 PM
In D47985#1094102, @aokblast wrote:

BTW, is hwpstate_amd quite older than intel one? I thought it don't have autonomous mode and it throttling all cpu into same freqs.

Indeed, it doesn't implement the CPPC interfaces. As far as I know, we just rely on the platform firmware to handle CPU power management.

markj accepted this revision.Mon, Dec 9, 3:11 PM
This revision is now accepted and ready to land.Mon, Dec 9, 3:11 PM
emaste added a subscriber: emaste.Mon, Dec 9, 9:04 PM
git_danielschaefer.me added a subscriber: git_danielschaefer.me.Mon, Dec 9, 11:52 PM

Tested together with aokblast yesterday on Framework 16 (Ryzen 4) and it seems to work with case 0x19:.
Frequency can't correctly be reported but from the temperature reading it seems that very likely the frequency is scaled.

Does frequency reporting work correctly on ryzen 5?

aokblast added a comment.EditedFri, Dec 13, 4:24 AM

It doesn't report the correct freq from zen5 (observed from htop). We can only observe a static frequency instead of dynamically changed. I will create a patch about zen4 later

Revision Contents
Changeset List

PathSize
sys/
x86/
cpufreq/
59 lines

Diff 147728

View Options

sys/x86/cpufreq/hwpstate_amd.c

Loading...