On (01/22/13 18:40), Austin Zhang wrote:
> Date: Tue, 22 Jan 2013 18:40:03 +0800
> From: Austin Zhang <zhang.austin@gmail.com>
> To: Sergey Senozhatsky <sergey.senozhatsky@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@gmail.com>
>> > From: Austin Zhang <[2]zhang.austin@gmail.com>
> On (01/15/13 19:05), Austin Zhang wrote:
> > Date: Tue, 15 Jan 2013 19:05:18 +0800
> > To: [3]powertop@lists.01.org
> > Subject: [Powertop] [PATCH] Fix C states and P states presentation> > Signed-off-by: Austin Zhang <[4]zhang.austin@gmail.com>
> 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.
> >
> > ---
> > �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@lists.01.org> 1. mailto:sergey.senozhatsky@gmail.com
> > [6]https://lists.01.org/mailman/listinfo/powertop
> >
>
> References
>
> Visible links
> 2. mailto:zhang.austin@gmail.com
> 3. mailto:powertop@lists.01.org
> 4. mailto:zhang.austin@gmail.com
> 5. mailto:PowerTop@lists.01.org
> 6. https://lists.01.org/mailman/listinfo/powertop