Messages in this thread | | | Subject | Re: [PATCH 1/8] cpupower: Update msr_pstate union struct naming | From | Shuah Khan <> | Date | Fri, 22 Jan 2021 13:17:17 -0700 |
| |
On 1/22/21 10:38 AM, Nathan Fontenot wrote: > The msr_pstate union struct named fam17h_bits is misleading since > this is the struct to use for all families >= 0x17, not just > for family 0x17. Rename the bits structs to be 'pstate' (for pre > family 17h CPUs) and 'pstatedef' (for CPUs since fam 17h) to align > closer with PPR/BDKG naming.
What is PPR/PDKG - would be helpful to know what it is and provide link to the doc if applicable. > > There are no functional changes as part of this update. > > Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com> > --- > tools/power/cpupower/utils/helpers/amd.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/tools/power/cpupower/utils/helpers/amd.c b/tools/power/cpupower/utils/helpers/amd.c > index 7c4f83a8c973..34368436bbd6 100644 > --- a/tools/power/cpupower/utils/helpers/amd.c > +++ b/tools/power/cpupower/utils/helpers/amd.c > @@ -13,7 +13,8 @@ > #define MSR_AMD_PSTATE 0xc0010064 > #define MSR_AMD_PSTATE_LIMIT 0xc0010061 > > -union msr_pstate { > +union core_pstate { > + /* pre fam 17h: */ > struct { > unsigned fid:6; > unsigned did:3; > @@ -26,7 +27,8 @@ union msr_pstate { > unsigned idddiv:2; > unsigned res3:21; > unsigned en:1; > - } bits; > + } pstate; > + /* since fam 17h: */ > struct { > unsigned fid:8; > unsigned did:6; > @@ -35,36 +37,36 @@ union msr_pstate { > unsigned idddiv:2; > unsigned res1:31; > unsigned en:1; > - } fam17h_bits; > + } pstatedef;
Does pstatedef indicate this is pstate default?
> unsigned long long val; > }; > > -static int get_did(int family, union msr_pstate pstate) > +static int get_did(int family, union core_pstate pstate) > { > int t; > > if (family == 0x12) > t = pstate.val & 0xf; > else if (family == 0x17 || family == 0x18) > - t = pstate.fam17h_bits.did; > + t = pstate.pstatedef.did; > else > - t = pstate.bits.did; > + t = pstate.pstate.did; > > return t; > } > > -static int get_cof(int family, union msr_pstate pstate) > +static int get_cof(int family, union core_pstate pstate) > { > int t; > int fid, did, cof; > > did = get_did(family, pstate); > if (family == 0x17 || family == 0x18) { > - fid = pstate.fam17h_bits.fid; > + fid = pstate.pstatedef.fid; > cof = 200 * fid / did; > } else { > t = 0x10; > - fid = pstate.bits.fid; > + fid = pstate.pstate.fid; > if (family == 0x11) > t = 0x8; > cof = (100 * (fid + t)) >> did; > @@ -89,7 +91,7 @@ int decode_pstates(unsigned int cpu, unsigned int cpu_family, > int boost_states, unsigned long *pstates, int *no) > { > int i, psmax, pscur; > - union msr_pstate pstate; > + union core_pstate pstate; > unsigned long long val; > > /* Only read out frequencies from HW when CPU might be boostable > @@ -119,9 +121,9 @@ int decode_pstates(unsigned int cpu, unsigned int cpu_family, > } > if (read_msr(cpu, MSR_AMD_PSTATE + i, &pstate.val)) > return -1; > - if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en)) > + if ((cpu_family == 0x17) && (!pstate.pstatedef.en)) > continue; > - else if (!pstate.bits.en) > + else if (!pstate.pstate.en) > continue; > > pstates[i] = get_cof(cpu_family, pstate); > >
thanks, -- Shuah
| |