* Re: [Powertop] Display all P-states in HTML-report
@ 2012-07-17 22:08 Chris Ferron
0 siblings, 0 replies; 5+ messages in thread
From: Chris Ferron @ 2012-07-17 22:08 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]
On 07/10/2012 11:13 PM, Igor Zhbanov wrote:
> Hello!
>
> On Arm processors there are more than 10 P-states. On my device there
> are 12
> p-states with different frequency. But PowerTOP displays only 10. And it
> doesn't display states (11 and 12) where the phone spends most of the
> time.
>
> I suppose that was because of console mode of PowerTOP, when there are
> limited
> number of lines on the console. But in the case of HTML-report this
> should
> not be a problem.
>
> Here is the patch that counts the number of found P-states and
> displays all
> of them.
>
> --8<--Cut
> here----------------------------------------------------------------
> diff -purN -X dontdiff-powertop powertop-intel/config.h powertop/config.h
> diff -purN -X dontdiff-powertop powertop-intel/src/cpu/cpu.cpp
> powertop/src/cpu/cpu.cpp
> --- powertop-intel/src/cpu/cpu.cpp 2012-07-05 13:06:16.000000000 +0400
> +++ powertop/src/cpu/cpu.cpp 2012-07-05 18:43:19.704809327 +0400
> @@ -618,6 +618,11 @@ void report_display_cpu_pstates(void)
> unsigned int package, core, cpu;
> int line;
> class abstract_cpu *_package, * _core, * _cpu;
> + unsigned int i, pstates_num;
> +
> + for (i = 0, pstates_num = 0; i< all_cpus.size(); i++)
> + if (all_cpus[i]&& all_cpus[i]->pstates.size()> pstates_num)
> + pstates_num = all_cpus[i]->pstates.size();
>
> if ((!reportout.csv_report)&&(!reportout.http_report))
> return;
> @@ -644,7 +649,7 @@ void report_display_cpu_pstates(void)
> if (!_core)
> continue;
>
> - for (line = LEVEL_HEADER; line< 10; line++) {
> + for (line = LEVEL_HEADER; line< (int)pstates_num; line++) {
> int first = 1;
>
> if (!_package->has_pstate_level(line))
> --8<--------------------------------------------------------------------------
>
>
> The same issue may affect the function report_display_cpu_cstates()
> but I'm not sure that CPUs with more than 10 C-states exists.
>
> The same code is in the function impl_w_display_cpu_states(), but that
> function is used in console mode only.
>
> Thank you.
>
>
>
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
Your patch has been merged.
Thank you,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Powertop] Display all P-states in HTML-report
@ 2012-07-12 0:57 Sergey Senozhatsky
0 siblings, 0 replies; 5+ messages in thread
From: Sergey Senozhatsky @ 2012-07-12 0:57 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 392 bytes --]
On (07/11/12 12:40), Igor Zhbanov wrote:
>
> + unsigned int i, pstates_num;
> +
> + for (i = 0, pstates_num = 0; i< all_cpus.size(); i++)
> + if (all_cpus[i]&& all_cpus[i]->pstates.size()> pstates_num)
> + pstates_num = all_cpus[i]->pstates.size();
>
OH, SILLY ME! My bad, sorry!
You initialize pstates_num within for loop (which is confusing).
Sorry for noise.
-ss
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Powertop] Display all P-states in HTML-report
@ 2012-07-12 0:54 Sergey Senozhatsky
0 siblings, 0 replies; 5+ messages in thread
From: Sergey Senozhatsky @ 2012-07-12 0:54 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 875 bytes --]
On (07/11/12 12:40), Igor Zhbanov wrote:
>
> + unsigned int i, pstates_num;
> +
> + for (i = 0, pstates_num = 0; i< all_cpus.size(); i++)
> + if (all_cpus[i]&& all_cpus[i]->pstates.size()> pstates_num)
> + pstates_num = all_cpus[i]->pstates.size();
>
nack
That will not work correctly, I'm afraid.
pstates_num initialized with stack garbage and is free to have value (e.g. 0x7fffffff)
which will never satisfy "all_cpus[i]->pstates.size()> pstates_num" condition.
In that case our loop will spin "0x7fffffff - actual_pstates_number" useless cycles.
BTW, could you please put white space before operators and remove extra one after?
E.g.
"all_cpus[i] && all_cpus[i]->pstates.size() > pstates_num"
vs.
"all_cpus[i]&& all_cpus[i]->pstates.size()> pstates_num"
^^^^^^ ^^^^^
-ss
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Powertop] Display all P-states in HTML-report
@ 2012-07-11 8:40 Igor Zhbanov
0 siblings, 0 replies; 5+ messages in thread
From: Igor Zhbanov @ 2012-07-11 8:40 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 4433 bytes --]
This is the formatted version of the patch:
From 66d0f194250d94afcf83eb7712af6f50c6ad8ec4 Mon Sep 17 00:00:00 2001
From: Igor Zhbanov<i.zhbanov(a)samsung.com>
Date: Wed, 11 Jul 2012 12:35:31 +0400
Subject: [PATCH] Display all P-states in HTML-report
On Arm processors there are more than 10 P-states. On my device there are 12
p-states with different frequency. But PowerTOP displays only 10. And it
doesn't display states (11 and 12) where the phone spends most of the time.
I suppose that was because of console mode of PowerTOP, when there are limited
number of lines on the console. But in the case of HTML-report this should
not be a problem.
This patch that counts the number of found P-states and displays all of them.
The same issue may affect the function report_display_cpu_cstates()
but I'm not sure that CPUs with more than 10 C-states exists.
The same code is in the function impl_w_display_cpu_states(), but that
function is used in console mode only where lines limit is reasonable.
---
src/cpu/cpu.cpp | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
index 4096835..c3dd6a5 100644
--- a/src/cpu/cpu.cpp
+++ b/src/cpu/cpu.cpp
@@ -588,6 +588,11 @@ void report_display_cpu_pstates(void)
unsigned int package, core, cpu;
int line;
class abstract_cpu *_package, * _core, * _cpu;
+ unsigned int i, pstates_num;
+
+ for (i = 0, pstates_num = 0; i< all_cpus.size(); i++)
+ if (all_cpus[i]&& all_cpus[i]->pstates.size()> pstates_num)
+ pstates_num = all_cpus[i]->pstates.size();
if ((!reportout.csv_report)&&(!reportout.http_report))
return;
@@ -614,7 +619,7 @@ void report_display_cpu_pstates(void)
if (!_core)
continue;
- for (line = LEVEL_HEADER; line< 10; line++) {
+ for (line = LEVEL_HEADER; line< (int)pstates_num; line++) {
int first = 1;
if (!_package->has_pstate_level(line))
--
1.7.5.4
Igor Zhbanov wrote:
> Hello!
>
> On Arm processors there are more than 10 P-states. On my device there
> are 12
> p-states with different frequency. But PowerTOP displays only 10. And it
> doesn't display states (11 and 12) where the phone spends most of the
> time.
>
> I suppose that was because of console mode of PowerTOP, when there are
> limited
> number of lines on the console. But in the case of HTML-report this
> should
> not be a problem.
>
> Here is the patch that counts the number of found P-states and
> displays all
> of them.
>
> --8<--Cut
> here----------------------------------------------------------------
> diff -purN -X dontdiff-powertop powertop-intel/config.h powertop/config.h
> diff -purN -X dontdiff-powertop powertop-intel/src/cpu/cpu.cpp
> powertop/src/cpu/cpu.cpp
> --- powertop-intel/src/cpu/cpu.cpp 2012-07-05 13:06:16.000000000 +0400
> +++ powertop/src/cpu/cpu.cpp 2012-07-05 18:43:19.704809327 +0400
> @@ -618,6 +618,11 @@ void report_display_cpu_pstates(void)
> unsigned int package, core, cpu;
> int line;
> class abstract_cpu *_package, * _core, * _cpu;
> + unsigned int i, pstates_num;
> +
> + for (i = 0, pstates_num = 0; i< all_cpus.size(); i++)
> + if (all_cpus[i]&& all_cpus[i]->pstates.size()> pstates_num)
> + pstates_num = all_cpus[i]->pstates.size();
>
> if ((!reportout.csv_report)&&(!reportout.http_report))
> return;
> @@ -644,7 +649,7 @@ void report_display_cpu_pstates(void)
> if (!_core)
> continue;
>
> - for (line = LEVEL_HEADER; line< 10; line++) {
> + for (line = LEVEL_HEADER; line< (int)pstates_num; line++) {
> int first = 1;
>
> if (!_package->has_pstate_level(line))
> --8<--------------------------------------------------------------------------
>
>
> The same issue may affect the function report_display_cpu_cstates()
> but I'm not sure that CPUs with more than 10 C-states exists.
>
> The same code is in the function impl_w_display_cpu_states(), but that
> function is used in console mode only.
>
> Thank you.
--
Best regards,
Igor Zhbanov,
Expert Software Engineer,
phone: +7 (495) 797 25 00 ext 3806
e-mail: i.zhbanov(a)samsung.com
ASWG, Moscow R&D center, Samsung Electronics
12 Dvintsev street, building 1
127018, Moscow, Russian Federation
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Powertop] Display all P-states in HTML-report
@ 2012-07-11 6:13 Igor Zhbanov
0 siblings, 0 replies; 5+ messages in thread
From: Igor Zhbanov @ 2012-07-11 6:13 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 2209 bytes --]
Hello!
On Arm processors there are more than 10 P-states. On my device there are 12
p-states with different frequency. But PowerTOP displays only 10. And it
doesn't display states (11 and 12) where the phone spends most of the time.
I suppose that was because of console mode of PowerTOP, when there are limited
number of lines on the console. But in the case of HTML-report this should
not be a problem.
Here is the patch that counts the number of found P-states and displays all
of them.
--8<--Cut here----------------------------------------------------------------
diff -purN -X dontdiff-powertop powertop-intel/config.h powertop/config.h
diff -purN -X dontdiff-powertop powertop-intel/src/cpu/cpu.cpp powertop/src/cpu/cpu.cpp
--- powertop-intel/src/cpu/cpu.cpp 2012-07-05 13:06:16.000000000 +0400
+++ powertop/src/cpu/cpu.cpp 2012-07-05 18:43:19.704809327 +0400
@@ -618,6 +618,11 @@ void report_display_cpu_pstates(void)
unsigned int package, core, cpu;
int line;
class abstract_cpu *_package, * _core, * _cpu;
+ unsigned int i, pstates_num;
+
+ for (i = 0, pstates_num = 0; i< all_cpus.size(); i++)
+ if (all_cpus[i]&& all_cpus[i]->pstates.size()> pstates_num)
+ pstates_num = all_cpus[i]->pstates.size();
if ((!reportout.csv_report)&&(!reportout.http_report))
return;
@@ -644,7 +649,7 @@ void report_display_cpu_pstates(void)
if (!_core)
continue;
- for (line = LEVEL_HEADER; line< 10; line++) {
+ for (line = LEVEL_HEADER; line< (int)pstates_num; line++) {
int first = 1;
if (!_package->has_pstate_level(line))
--8<--------------------------------------------------------------------------
The same issue may affect the function report_display_cpu_cstates()
but I'm not sure that CPUs with more than 10 C-states exists.
The same code is in the function impl_w_display_cpu_states(), but that
function is used in console mode only.
Thank you.
--
Best regards,
Igor Zhbanov,
Expert Software Engineer,
phone: +7 (495) 797 25 00 ext 3806
e-mail: i.zhbanov(a)samsung.com
ASWG, Moscow R&D center, Samsung Electronics
12 Dvintsev street, building 1
127018, Moscow, Russian Federation
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 04-powertop-fix02.diff --]
[-- Type: text/x-diff, Size: 998 bytes --]
diff -purN -X dontdiff-powertop powertop-intel/config.h powertop/config.h
diff -purN -X dontdiff-powertop powertop-intel/src/cpu/cpu.cpp powertop/src/cpu/cpu.cpp
--- powertop-intel/src/cpu/cpu.cpp 2012-07-05 13:06:16.000000000 +0400
+++ powertop/src/cpu/cpu.cpp 2012-07-05 18:43:19.704809327 +0400
@@ -618,6 +618,11 @@ void report_display_cpu_pstates(void)
unsigned int package, core, cpu;
int line;
class abstract_cpu *_package, * _core, * _cpu;
+ unsigned int i, pstates_num;
+
+ for (i = 0, pstates_num = 0; i < all_cpus.size(); i++)
+ if (all_cpus[i] && all_cpus[i]->pstates.size() > pstates_num)
+ pstates_num = all_cpus[i]->pstates.size();
if ((!reportout.csv_report)&&(!reportout.http_report))
return;
@@ -644,7 +649,7 @@ void report_display_cpu_pstates(void)
if (!_core)
continue;
- for (line = LEVEL_HEADER; line < 10; line++) {
+ for (line = LEVEL_HEADER; line < (int)pstates_num; line++) {
int first = 1;
if (!_package->has_pstate_level(line))
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-17 22:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-17 22:08 [Powertop] Display all P-states in HTML-report Chris Ferron
-- strict thread matches above, loose matches on Subject: below --
2012-07-12 0:57 Sergey Senozhatsky
2012-07-12 0:54 Sergey Senozhatsky
2012-07-11 8:40 Igor Zhbanov
2012-07-11 6:13 Igor Zhbanov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.