| Message ID | 20210503192810.36084-6-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Intel Hardware P-States (HWP) support | expand |
On 03.05.2021 21:28, Jason Andryuk wrote: > --- a/tools/misc/xenpm.c > +++ b/tools/misc/xenpm.c > @@ -711,6 +711,7 @@ void start_gather_func(int argc, char *argv[]) > /* print out parameters about cpu frequency */ > static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq) > { > + bool internal = strstr(p_cpufreq->scaling_governor, "internal"); Like suggested for the hypervisor, perhaps better check for names ending in "-internal"? > @@ -720,10 +721,19 @@ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq) > printf(" %d", p_cpufreq->affected_cpus[i]); > printf("\n"); > > - printf("cpuinfo frequency : max [%u] min [%u] cur [%u]\n", > - p_cpufreq->cpuinfo_max_freq, > - p_cpufreq->cpuinfo_min_freq, > - p_cpufreq->cpuinfo_cur_freq); > + if ( internal ) > + { > + printf("cpuinfo frequency : base [%u] turbo [%u]\n", > + p_cpufreq->cpuinfo_min_freq, > + p_cpufreq->cpuinfo_max_freq); > + } > + else > + { > + printf("cpuinfo frequency : max [%u] min [%u] cur [%u]\n", > + p_cpufreq->cpuinfo_max_freq, > + p_cpufreq->cpuinfo_min_freq, > + p_cpufreq->cpuinfo_cur_freq); > + } Since the file adopts hypervisor coding style, the unnecessary braces would again better be omitted. Jan
On 26.05.2021 17:21, Jan Beulich wrote: > On 03.05.2021 21:28, Jason Andryuk wrote: >> --- a/tools/misc/xenpm.c >> +++ b/tools/misc/xenpm.c >> @@ -711,6 +711,7 @@ void start_gather_func(int argc, char *argv[]) >> /* print out parameters about cpu frequency */ >> static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq) >> { >> + bool internal = strstr(p_cpufreq->scaling_governor, "internal"); > > Like suggested for the hypervisor, perhaps better check for names > ending in "-internal"? Thinking about it more, the adjustments you make aren't necessarily applicable to other hypothetical internal governors, are they? In which case you rather want to check for hwp specifically. Jan
diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c index d0191d4984..562bf939f9 100644 --- a/tools/misc/xenpm.c +++ b/tools/misc/xenpm.c @@ -711,6 +711,7 @@ void start_gather_func(int argc, char *argv[]) /* print out parameters about cpu frequency */ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq) { + bool internal = strstr(p_cpufreq->scaling_governor, "internal"); int i; printf("cpu id : %d\n", cpuid); @@ -720,10 +721,19 @@ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq) printf(" %d", p_cpufreq->affected_cpus[i]); printf("\n"); - printf("cpuinfo frequency : max [%u] min [%u] cur [%u]\n", - p_cpufreq->cpuinfo_max_freq, - p_cpufreq->cpuinfo_min_freq, - p_cpufreq->cpuinfo_cur_freq); + if ( internal ) + { + printf("cpuinfo frequency : base [%u] turbo [%u]\n", + p_cpufreq->cpuinfo_min_freq, + p_cpufreq->cpuinfo_max_freq); + } + else + { + printf("cpuinfo frequency : max [%u] min [%u] cur [%u]\n", + p_cpufreq->cpuinfo_max_freq, + p_cpufreq->cpuinfo_min_freq, + p_cpufreq->cpuinfo_cur_freq); + } printf("scaling_driver : %s\n", p_cpufreq->scaling_driver); @@ -750,19 +760,22 @@ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq) p_cpufreq->u.ondemand.up_threshold); } - printf("scaling_avail_freq :"); - for ( i = 0; i < p_cpufreq->freq_num; i++ ) - if ( p_cpufreq->scaling_available_frequencies[i] == - p_cpufreq->scaling_cur_freq ) - printf(" *%d", p_cpufreq->scaling_available_frequencies[i]); - else - printf(" %d", p_cpufreq->scaling_available_frequencies[i]); - printf("\n"); + if ( !internal ) + { + printf("scaling_avail_freq :"); + for ( i = 0; i < p_cpufreq->freq_num; i++ ) + if ( p_cpufreq->scaling_available_frequencies[i] == + p_cpufreq->scaling_cur_freq ) + printf(" *%d", p_cpufreq->scaling_available_frequencies[i]); + else + printf(" %d", p_cpufreq->scaling_available_frequencies[i]); + printf("\n"); - printf("scaling frequency : max [%u] min [%u] cur [%u]\n", - p_cpufreq->scaling_max_freq, - p_cpufreq->scaling_min_freq, - p_cpufreq->scaling_cur_freq); + printf("scaling frequency : max [%u] min [%u] cur [%u]\n", + p_cpufreq->scaling_max_freq, + p_cpufreq->scaling_min_freq, + p_cpufreq->scaling_cur_freq); + } printf("turbo mode : %s\n", p_cpufreq->turbo_enabled ? "enabled" : "disabled or n/a");
When using HWP, some of the returned data is not applicable. In that case, we should just omit it to avoid confusing the user. So switch to printing the base and turbo frequencies since those are relevant to HWP. Similarly, stop printing the CPU frequencies since those do not apply. Signed-off-by: Jason Andryuk <jandryuk@gmail.com> --- tools/misc/xenpm.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-)