Re: [Powertop] [PATCH] Fix C states and P states presentation issue.

* Re: [Powertop] [PATCH] Fix C states and P states presentation issue.
@ 2013-01-22 11:45 Austin Zhang
  0 siblings, 0 replies; 7+ messages in thread
From: Austin Zhang @ 2013-01-22 11:45 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 8774 bytes --]

Hi, list,

Just find I just click 'reply-to' to ss, and doesn't cc/to
powertop(a)lists.01.org in this thread for those before feedback, sorry for
this.

Copy to here so that maybe Igor has comments also:

>Hm, I'm sure I've seen this before. Was it the same patch from you?
No, I didn't watch those patches from mailling list, just found this issue
in my daily working and then work out this patch. And I also didn't send it
before.

>Ok, I found it: "Display all P-states in HTML-report"
(http://lists.01.org/pipermail/powertop/2012-July/000161.html).
Thanks, just read it, it is only for pstates and this patch is learning the
same way from the mentioned codes. Seemed Igor and I encounter the same
situation for ARM and IA respectively :-)
Is the 2 patches conflict? I use the latest codes base from github.
Or the same fixing from igor is pending in the pool?

>but I'm not sure that CPUs with more than 10 C-states exists.
Yes, I am not sure the ARM case like igor is facing, for IA, yes.

2013/1/22 Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>

> On (01/22/13 18:40), Austin Zhang wrote:
> > Date: Tue, 22 Jan 2013 18:40:03 +0800
> > From: Austin Zhang <zhang.austin(a)gmail.com>
> > To: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
> > Subject: Re: [Powertop] [PATCH] Fix C states and P states presentation
> >  issue.
> >
> >    Sergey,
> >    Thanks for your review and comments, I will send out V2 by this. �
> >
>
> thank you,
>
>         -ss
>
>
> >    2013/1/22 Sergey Senozhatsky <[1]sergey.senozhatsky(a)gmail.com>
> >
> >      On (01/15/13 19:05), Austin Zhang wrote:
> >      > Date: Tue, 15 Jan 2013 19:05:18 +0800
> >      > From: Austin Zhang <[2]zhang.austin(a)gmail.com>
> >      > To: [3]powertop(a)lists.01.org
> >      > Subject: [Powertop] [PATCH] Fix C states and P states presentation
> >      issue.
> >      > X-Mailer: git-send-email 1.7.5.4
> >      >
> >      > If using '10' in this 'for' loop, for P states, once the processor
> >      > provides more than 10 P states, those additional ones cannot been
> >      > presented; and for C states, once the processor provides maximize
> >      > C states level more than 9, for example, it only has POLL, C1, C3,
> >      > C6, C10, the additional one like C10 also cannot be presented.
> >      >
> >      > Now we get the loop numbers from the system information we had got
> >      > before.
> >      >
> >      > Signed-off-by: Austin Zhang <[4]zhang.austin(a)gmail.com>
> >      > ---
> >      > �src/cpu/cpu.cpp | � 35 +++++++++++++++++++++++++++++------
> >      > �1 files changed, 29 insertions(+), 6 deletions(-)
> >      >
> >      > diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
> >      > index 7f3af69..28f77b7 100644
> >      > --- a/src/cpu/cpu.cpp
> >      > +++ b/src/cpu/cpu.cpp
> >      > @@ -404,6 +404,15 @@ void report_display_cpu_cstates(void)
> >      > � � � unsigned int package, core, cpu;
> >      > � � � int line;
> >      > � � � class abstract_cpu *_package, * _core, * _cpu;
> >      > + � � unsigned int i, j, cstates_num;
> >      > +
> >      > + � � for (i = 0, cstates_num = 0; i < all_cpus.size(); i++) {
> >      > + � � � � � � if (all_cpus[i])
> >      > + � � � � � � � � � � for (j = 0; j < all_cpus[i]->cstates.size();
> >      j++)
> >      > + � � � � � � � � � � � � � � if
> >      ((all_cpus[i]->cstates[j])->line_level >= 0 &&
> >      > + � � � � � � � � � � � � � � � �
> >      (all_cpus[i]->cstates[j])->line_level > cstates_num)
> >      > + � � � � � � � � � � � � � � � � � � cstates_num =
> >      (all_cpus[i]->cstates[j])->line_level;
> >      > + � � }
> >      >
> >
> >      how about removing this `if' condition?
> >      � � � � for (j = 0; j < all_cpus[i]->cstates.size(); j++)
> >      � � � � � � � � cstates_num = std::max(cstates_num,
> >      (all_cpus[i]->cstates[j])->line_level);
> >
> >      > � � � report.begin_section(SECTION_CPUIDLE);
> >      > � � � report.add_header("Processor Idle state report");
> >      > @@ -421,7 +430,7 @@ void report_display_cpu_cstates(void)
> >      > � � � � � � � � � � � if (!_core)
> >      > � � � � � � � � � � � � � � � continue;
> >      >
> >      > - � � � � � � � � � � for (line = LEVEL_HEADER; line < 10;
> line++) {
> >      > + � � � � � � � � � � for (line = LEVEL_HEADER; line <=
> >      (int)cstates_num; line++) {
> >      > � � � � � � � � � � � � � � � bool first_cpu = true;
> >      >
> >      > � � � � � � � � � � � � � � � if
> (!_package->has_cstate_level(line))
> >      > @@ -628,14 +637,29 @@ void impl_w_display_cpu_states(int state)
> >      > � � � char buffer[128];
> >      > � � � char linebuf[1024];
> >      > � � � unsigned int package, core, cpu;
> >      > - � � int line;
> >      > + � � int line, loop;
> >      > � � � class abstract_cpu *_package, * _core, * _cpu;
> >      > � � � int ctr = 0;
> >      > + � � unsigned int i, j, cstates_num, pstates_num;
> >      > +
> >      > + � � for (i = 0, cstates_num = 0, pstates_num = 0; i <
> >      all_cpus.size(); i++) {
> >
> >      we have two all_cpus[i] checks in one for loop, let's change to
> single
> >      `if (!all_cpus[i]) continue;'
> >
> >      > + � � � � � � if (all_cpus[i] && all_cpus[i]->pstates.size() >
> >      pstates_num)
> >      > + � � � � � � � � � � pstates_num = all_cpus[i]->pstates.size();
> >      >
> >
> >      � � � � pstates_num = std::max(pstates_num,
> all_cpus[i]->pstates.size())
> >
> >      > - � � if (state == PSTATE)
> >      > + � � � � � � if (all_cpus[i])
> >      > + � � � � � � � � � � for (j = 0; j < all_cpus[i]->cstates.size();
> >      j++)
> >      > + � � � � � � � � � � � � � � if
> >      ((all_cpus[i]->cstates[j])->line_level >= 0 &&
> >      > + � � � � � � � � � � � � � � � �
> >      (all_cpus[i]->cstates[j])->line_level > cstates_num)
> >      > + � � � � � � � � � � � � � � � � � � cstates_num =
> >      (all_cpus[i]->cstates[j])->line_level;
> >
> >      � � � � for (j = 0; j < all_cpus[i]->cstates.size(); j++)
> >      � � � � � � � � cstates_num = std::max(cstates_num,
> >      (all_cpus[i]->cstates[j])->line_level)
> >
> >      � � � � -ss
> >
> >      > + � � }
> >      > +
> >      > + � � if (state == PSTATE) {
> >      > � � � � � � � win = get_ncurses_win("Frequency stats");
> >      > - � � else
> >      > + � � � � � � loop = (int)pstates_num;
> >      > + � � } else {
> >      > � � � � � � � win = get_ncurses_win("Idle stats");
> >      > + � � � � � � loop = (int)cstates_num;
> >      > + � � }
> >      >
> >      > � � � if (!win)
> >      > � � � � � � � return;
> >      > @@ -656,8 +680,7 @@ void impl_w_display_cpu_states(int state)
> >      > � � � � � � � � � � � if (!_core->has_pstates() && state ==
> PSTATE)
> >      > � � � � � � � � � � � � � � � continue;
> >      >
> >      > -
> >      > - � � � � � � � � � � for (line = LEVEL_HEADER; line < 10;
> line++) {
> >      > + � � � � � � � � � � for (line = LEVEL_HEADER; line <= loop;
> line++)
> >      {
> >      > � � � � � � � � � � � � � � � int first = 1;
> >      > � � � � � � � � � � � � � � � ctr = 0;
> >      > � � � � � � � � � � � � � � � linebuf[0] = 0;
> >      > --
> >      > 1.7.5.4
> >      >
> >      > _______________________________________________
> >      > PowerTop mailing list
> >      > [5]PowerTop(a)lists.01.org
> >      > [6]https://lists.01.org/mailman/listinfo/powertop
> >      >
> >
> > References
> >
> >    Visible links
> >    1. mailto:sergey.senozhatsky(a)gmail.com
> >    2. mailto:zhang.austin(a)gmail.com
> >    3. mailto:powertop(a)lists.01.org
> >    4. mailto:zhang.austin(a)gmail.com
> >    5. mailto:PowerTop(a)lists.01.org
> >    6. https://lists.01.org/mailman/listinfo/powertop
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 12329 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread