Re: [RFC PATCH] tools/power turbostat: Add support for Hygon Fam 18h (Dhyana) RAPL

[Calvin Walton]

On Fri, 2019-08-30 at 17:23 +0800, Pu Wen wrote:
> Commit 9392bd98bba760be96ee ("tools/power turbostat: Add support for
> AMD
> Fam 17h (Zen) RAPL") and the commit 3316f99a9f1b68c578c5
> ("tools/power
> turbostat: Also read package power on AMD F17h (Zen)") add AMD Fam
> 17h
> RAPL support.
> 
> Hygon Family 18h(Dhyana) support RAPL in bit 14 of CPUID 0x80000007
> EDX,
> and has MSRs RAPL_PWR_UNIT/CORE_ENERGY_STAT/PKG_ENERGY_STAT. So add
> Hygon
> Dhyana Family 18h support for RAPL.
> 
> Already tested on Hygon multi-node systems and it shows correct per-
> core
> energy usage and the total package power.

I was a bit worried about these two chunks, since as far as I know AMD
has not yet released any processor with family 0x18, so there might be
future conflicts:

> @@ -3803,6 +3804,7 @@ double get_tdp_amd(unsigned int family)
>  {
>  	switch (family) {
>  	case 0x17:
> +	case 0x18:
>  	default:
> 

> @@ -3982,6 +3984,7 @@ void rapl_probe_amd(unsigned int family,
> unsigned int model)
>  
>  	switch (family) {
>  	case 0x17: /* Zen, Zen+ */
> +	case 0x18:
>  		do_rapl = RAPL_AMD_F17H | RAPL_PER_CORE_ENERGY;
>  		if (rapl_joules) {
>  			BIC_PRESENT(BIC_Pkg_J);

But the second switch is already guarded by the CPUID check for the
rapl support, so it will either "just work" if AMD's family 0x18 chip
uses the same RAPL registers - or cleanly skip the CPU if they changed
it.

Please add a comment on the 0x18 case in the rapl_probe_amd function,
something like:
	case 0x18: /* Hygon Dhyana */

Feel free to add a
Reviewed-by: Calvin Walton <calvin.walton@xxxxxxxxxx>

-- 
Calvin Walton <calvin.walton@xxxxxxxxxx>