From: Joe Konno <joe.konno(a)intel.com&gt; Depending on the sample time (-t sample_time_in_secs), on Intel CPUs the average frequency is computed for the sample time. Therefore, let's change the label to "Average" so as not to confuse the end-user. Previously, the UI and report printed the "actual" frequency-- at least, that's how it was labeled. This is not strictly true, as the actual cpu frequency of the platform may change at any point during the sample time for a variety of reasons. Signed-off-by: Joe Konno <joe.konno(a)intel.com&gt; --- src/cpu/intel_cpus.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp index 7cb9d45297ef..8151292238ec 100644 --- a/src/cpu/intel_cpus.cpp +++ b/src/cpu/intel_cpus.cpp @@ -573,7 +573,7 @@ void nhm_cpu::measurement_end(void) char * nhm_cpu::fill_pstate_name(int line_nr, char *buffer) { if (line_nr == LEVEL_C0) { - sprintf(buffer, _("Actual")); + sprintf(buffer, _("Average")); return buffer; } return cpu_linux::fill_pstate_name(line_nr, buffer); -- 2.8.3

Hi Joe, I just realized a further scenario that would be problematic with your current patch: once I'd read from Len Brown's post to Red Hat, that enabling HWP in intel_pstate driver is considered a mistake, because enabling HWP only need to write a few msr bits, based on a few simple policies, there's no need to load a whole intel_pstate driver to do that. so future upstream kernel would remove that part, and HWP enabling will be put to some CPPC driver, and on HWP reinforced platform, intel_pstate driver will not be loaded at all. So, if that happen, the is_intel_pstate_driver_loaded() will return false, and code will go to the old path, but the frequency statistics still not exist, then we got nothing. mmm... maybe you could use 'cpufreq statistics exists or not' to branch your code, if exist, use it; if not, calculate average freq from msrs. there should have been such a function to check that condition. -Wang, Xiaolong -----Original Message----- From: PowerTop [mailto:powertop-bounces@lists.01.org] On Behalf Of Joe Konno Sent: Tuesday, June 14, 2016 12:01 AM To: powertop(a)lists.01.org Subject: [Powertop] [PATCH 2/2] intel_cpus: no pstate Idle pct with intel_pstate From: Joe Konno <joe.konno(a)intel.com&gt; Do not display "percent Idle" in the Frequency Stats tab when the intel_pstate kernel driver is loaded. This is because Idle time is inferred through kernel PM trace events, which may be insufficient for proper computation in certain scenarios. Therefore, best to display no data than potentially inaccurate data. End-users are encouraged to use the Idle Stats tab in the UI or report to analyze their system's idle time. When the intel_pstate kernel driver is loaded, the Frequency Stats tab will only present the end-user with the average frequency of each logical core. Signed-off-by: Joe Konno <joe.konno(a)intel.com&gt; --- src/cpu/intel_cpus.cpp | 50 +++++++++++++++++++++++++++++++++++++++++++------- src/cpu/intel_cpus.h | 2 ++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp index 8151292238ec..330050e37473 100644 --- a/src/cpu/intel_cpus.cpp +++ b/src/cpu/intel_cpus.cpp @@ -70,6 +70,8 @@ static int intel_cpu_models[] = { 0 /* last entry must be zero */ }; +static int intel_pstate_driver_loaded = -1; + int is_supported_intel_cpu(int model) { int i; @@ -81,6 +83,34 @@ int is_supported_intel_cpu(int model) return 0; } +int is_intel_pstate_driver_loaded() +{ + const string filename("/sys/devices/system/cpu/cpu0/cpufreq/scaling_driver"); + const string intel_pstate("intel_pstate"); + char line[32] = { '\0' }; + ifstream file; + + if (intel_pstate_driver_loaded > -1) + return intel_pstate_driver_loaded; + + file.open(filename, ios::in); + + if (!file) + return -1; + + file.getline(line, sizeof(line)-1); + file.close(); + + const string scaling_driver(line); + if (scaling_driver == intel_pstate) { + intel_pstate_driver_loaded = 1; + } else { + intel_pstate_driver_loaded = 0; + } + + return intel_pstate_driver_loaded; +} + static uint64_t get_msr(int cpu, uint64_t offset) { ssize_t retval; @@ -267,10 +297,11 @@ void nhm_core::measurement_end(void) char * nhm_core::fill_pstate_line(int line_nr, char *buffer) { + const int intel_pstate = is_intel_pstate_driver_loaded(); buffer[0] = 0; unsigned int i; - if (total_stamp ==0) { + if (!intel_pstate && total_stamp ==0) { for (i = 0; i < pstates.size(); i++) total_stamp += pstates[i]->time_after; if (total_stamp == 0) @@ -282,10 +313,11 @@ char * nhm_core::fill_pstate_line(int line_nr, char *buffer) return buffer; } - if (line_nr >= (int)pstates.size() || line_nr < 0) + if (intel_pstate > 0 || line_nr >= (int)pstates.size() || line_nr < 0) return buffer; sprintf(buffer," %5.1f%% ", percentage(1.0* (pstates[line_nr]->time_after) / total_stamp)); + return buffer; } @@ -342,10 +374,11 @@ nhm_package::nhm_package(int model) char * nhm_package::fill_pstate_line(int line_nr, char *buffer) { + const int intel_pstate = is_intel_pstate_driver_loaded(); buffer[0] = 0; unsigned int i; - if (total_stamp ==0) { + if (!intel_pstate && total_stamp ==0) { for (i = 0; i < pstates.size(); i++) total_stamp += pstates[i]->time_after; if (total_stamp == 0) @@ -358,10 +391,11 @@ char * nhm_package::fill_pstate_line(int line_nr, char *buffer) return buffer; } - if (line_nr >= (int)pstates.size() || line_nr < 0) + if (intel_pstate > 0 || line_nr >= (int)pstates.size() || line_nr < 0) return buffer; sprintf(buffer," %5.1f%% ", percentage(1.0* (pstates[line_nr]->time_after) / total_stamp)); + return buffer; } @@ -581,7 +615,9 @@ char * nhm_cpu::fill_pstate_name(int line_nr, char *buffer) char * nhm_cpu::fill_pstate_line(int line_nr, char *buffer) { - if (total_stamp ==0) { + const int intel_pstate = is_intel_pstate_driver_loaded(); + + if (!intel_pstate && total_stamp ==0) { unsigned int i; for (i = 0; i < pstates.size(); i++) total_stamp += pstates[i]->time_after; @@ -600,12 +636,12 @@ char * nhm_cpu::fill_pstate_line(int line_nr, char *buffer) hz_to_human(F, buffer, 1); return buffer; } - if (line_nr >= (int)pstates.size() || line_nr < 0) + if (intel_pstate > 0 || line_nr >= (int)pstates.size() || line_nr < 0) return buffer; sprintf(buffer," %5.1f%% ", percentage(1.0* (pstates[line_nr]->time_after) / total_stamp)); - return buffer; + return buffer; } diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h index 03310692a83b..d20db9ae3986 100644 --- a/src/cpu/intel_cpus.h +++ b/src/cpu/intel_cpus.h @@ -175,4 +175,6 @@ public: int is_supported_intel_cpu(int model); int byt_has_ahci(); +int is_intel_pstate_driver_loaded(); + #endif -- 2.8.3 _______________________________________________ PowerTop mailing list PowerTop(a)lists.01.org https://lists.01.org/mailman/listinfo/powertop