All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Powertop] [PATCH v3 18/31] report: CPU Frequency html & csv
@ 2013-12-10 20:12 Sergey Senozhatsky
  0 siblings, 0 replies; 6+ messages in thread
From: Sergey Senozhatsky @ 2013-12-10 20:12 UTC (permalink / raw)
  To: powertop

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

On (11/18/13 12:27), Alexandra Yates wrote:
> Enables html and csv report new stlyes for "CPU Frequency" section,
> While preserving translation macros.
> 
> Signed-off-by: Alexandra Yates <alexandra.yates(a)linux.intel.com>
> ---
>  src/cpu/cpu.cpp |  139 ++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 101 insertions(+), 38 deletions(-)
> 
> diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
> index b8f74a8..c3787e6 100644
> --- a/src/cpu/cpu.cpp
> +++ b/src/cpu/cpu.cpp
> @@ -612,29 +612,81 @@ void report_display_cpu_cstates(void)
>  
>  void report_display_cpu_pstates(void)
>  {
> -	char buffer[512], buffer2[512];
> +	char buffer[512], buffer2[512], tmp_num[50];
>  	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++)
> +	/* div attr css_class and css_id */
> +	tag_attr div_attr;
> +	init_div(&div_attr, "clear_block", "cpufreq");
> +
> +	/* Set Table attributes, rows, and cols */
> +	table_attributes std_table_css;
> +	table_size pkg_tbl_size;
> +	table_size core_tbl_size;
> +	table_size cpu_tbl_size;
> +
> +
> +	/* Set Title attributes */
> +	tag_attr title_attr;
> +	init_title_attr(&title_attr);
> +
> +	/* Report add section */
> +        report.add_div(&div_attr);
> +        report.add_title(&title_attr, __("Processor Frequency Report"));
> +
> +        /* Set array of data in row Major order */
> +	int idx1, idx2, idx3, num_cpus=0;
> +        string tmp_str;
> +
> +	for (i = 0, pstates_num = 0; i < all_cpus.size(); i++) {
>  		if (all_cpus[i])
>  			pstates_num = std::max<unsigned int>(pstates_num,
> -								all_cpus[i]->pstates.size());
> -
> -	report.begin_section(SECTION_CPUFREQ);
> -	report.add_header("Processor Frequency Report");
> +				all_cpus[i]->pstates.size());
> +	}
>  
>  	for (package = 0; package < system_level.children.size(); package++) {
>  		bool first_core = true;
> +		idx1=0;
> +		idx2=0;
> +		idx3=0;
>  
>  		_package = system_level.children[package];
>  		if (!_package)
>  			continue;
>  
> -		report.begin_table(TABLE_WIDE);
> +		/*  Tables for PKG, CORE, CPU */
> +		pkg_tbl_size.cols=2;
> +		pkg_tbl_size.rows=pstates_num+2;
> +		string pkg_data[pkg_tbl_size.cols * pkg_tbl_size.rows];
> +
> +		core_tbl_size.cols=2;
> +		core_tbl_size.rows=((pstates_num+2) *_package->children.size());
> +		string core_data[core_tbl_size.cols * core_tbl_size.rows];
> +
> +		/* PKG */
> +		num_cpus=0;
> +		for (core = 0; core < _package->children.size(); core++) {
> +			_core = _package->children[core];
> +			if (!_core)
> +				continue;
> +			for (cpu = 0; cpu < _core->children.size(); cpu++) {
> +				if (!_cpu)
> +				continue;
> +				_cpu = _core->children[cpu];
> +				num_cpus+=1;
> +			}
> +		}
> +		cpu_tbl_size.cols= num_cpus+1;
> +		cpu_tbl_size.rows= (pstates_num+2)* _package->children.size();
> +		string cpu_data[cpu_tbl_size.cols * cpu_tbl_size.rows];
> +
> +		/* Core */
>  		for (core = 0; core < _package->children.size(); core++) {
> +			cpu_data[idx3]="&nbsp;";
> +			idx3+=1;
>  			_core = _package->children[core];
>  			if (!_core)
>  				continue;
> @@ -646,75 +698,86 @@ void report_display_cpu_pstates(void)
>  				bool first_cpu = true;
>  
>  				if (!_package->has_pstate_level(line))
> -					continue;
> -
> -				report.begin_row();
> +				continue;
>  
>  				buffer[0] = 0;
>  				buffer2[0] = 0;
>  				if (first_core) {
>  					if (line == LEVEL_HEADER) {
> -						report.begin_cell(CELL_FIRST_PACKAGE_HEADER);
> -						report.addf(__("Package %i"), _package->get_number());
> +						pkg_data[idx1]=__("Package");
> +						idx1+=1;
> +						sprintf(tmp_num,"%d",  _package->get_number());
> +						pkg_data[idx1]= string(tmp_num);
> +						idx1+=1;


initialisation:

       report.begin_section(SECTION_CPUFREQ);
       report.add_header("Processor Frequency Report");

       report.begin_table(TABLE_WIDE);

vs

       /* div attr css_class and css_id */
       tag_attr div_attr;
       init_div(&div_attr, "clear_block", "cpufreq");

       /* Set Table attributes, rows, and cols */
       table_attributes std_table_css;
       table_size pkg_tbl_size;
       table_size core_tbl_size;
       table_size cpu_tbl_size;


       /* Set Title attributes */
       tag_attr title_attr;
       init_title_attr(&title_attr);

       /* Report add section */
        report.add_div(&div_attr);
        report.add_title(&title_attr, __("Processor Frequency Report"));

        /* Set array of data in row Major order */
       int idx1, idx2, idx3, num_cpus=0;
        string tmp_str;





reporter usage:


         report.begin_cell(CELL_CORE_HEADER);
         report.addf(__("Core %i"), _core->get_number());

vs

         core_data[idx2]="";
         idx2+=1;
         sprintf(tmp_num,__("Core %d"),_core->get_number());
         core_data[idx2]=string(tmp_num);
         idx2+=1;





and





         report.begin_cell(CELL_STATE_NAME);
         report.add(_core->fill_pstate_name(line, buffer));
         report.begin_cell(CELL_PACKAGE_STATE_VALUE);
         report.add(_core->fill_pstate_line(line, buffer2));

vs

         tmp_str=string(_core->fill_pstate_name(line, buffer));
         core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
         idx2+=1;
         tmp_str=string(_core->fill_pstate_line(line, buffer2));
         core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
         idx2+=1;


sorry, but the existing one really looks easier to follow and use. current reporter user
cares less about reporter internall details.

	-ss


>  					} else {
> -						report.begin_cell(CELL_STATE_NAME);
> -						report.add(_package->fill_pstate_name(line, buffer));
> -						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
> -						report.add(_package->fill_pstate_line(line, buffer2));
> +						tmp_str=string(_package->fill_pstate_name(line, buffer));
> +						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
> +						idx1+=1;
> +						tmp_str=string(_package->fill_pstate_line(line, buffer2));
> +						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
> +						idx1+=1;
>  					}
> -				} else {
> -					report.begin_cell(CELL_EMPTY_PACKAGE_STATE);
> -					report.add_empty_cell();
>  				}
>  
> -				report.begin_cell(CELL_SEPARATOR);
> -				report.add_empty_cell();
>  
>  				if (!_core->can_collapse()) {
>  					buffer[0] = 0;
>  					buffer2[0] = 0;
>  					if (line == LEVEL_HEADER) {
> -						report.begin_cell(CELL_CORE_HEADER);
> -						report.addf(__("Core %i"), _core->get_number());
> +						core_data[idx2]="";
> +						idx2+=1;
> +						sprintf(tmp_num,__("Core %d"),_core->get_number());
> +						core_data[idx2]=string(tmp_num);
> +						idx2+=1;
>  					} else {
> -						report.begin_cell(CELL_STATE_NAME);
> -						report.add(_core->fill_pstate_name(line, buffer));
> -						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
> -						report.add(_core->fill_pstate_line(line, buffer2));
> +						tmp_str=string(_core->fill_pstate_name(line, buffer));
> +						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
> +						idx2+=1;
> +						tmp_str=string(_core->fill_pstate_line(line, buffer2));
> +						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
> +						idx2+=1;
>  					}
>  				}
>  
> -				report.begin_cell(CELL_SEPARATOR);
> -				report.add_empty_cell();
> -
> +				/* CPU */
>  				for (cpu = 0; cpu < _core->children.size(); cpu++) {
>  					buffer[0] = 0;
>  					_cpu = _core->children[cpu];
>  					if (!_cpu)
>  						continue;
>  
> -					report.set_cpu_number(cpu);
>  					if (line == LEVEL_HEADER) {
> -						report.begin_cell(CELL_CPU_PSTATE_HEADER);
> -						report.addf(__("CPU %i"), _cpu->get_number());
> +						sprintf(tmp_num,__("CPU %d"),_cpu->get_number());
> +						cpu_data[idx3] = string(tmp_num);
> +						idx3+=1;
>  						continue;
>  					}
>  
>  					if (first_cpu) {
> -						report.begin_cell(CELL_STATE_NAME);
> -						report.add(_cpu->fill_pstate_name(line, buffer));
> +						tmp_str=string(_cpu->fill_pstate_name(line, buffer));
> +						cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
> +						idx3+=1;
>  						first_cpu = false;
>  					}
>  
>  					buffer[0] = 0;
> -					report.begin_cell(CELL_CPU_STATE_VALUE);
> -					report.add(_cpu->fill_pstate_line(line, buffer));
> +					tmp_str=string(_cpu->fill_pstate_line(line, buffer));
> +					cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
> +					idx3+=1;
>  				}
>  			}
> -
>  			first_core = false;
>  		}
> +		init_pkg_table_attr(&std_table_css, pkg_tbl_size.rows, pkg_tbl_size.cols);
> +		report.add_table(pkg_data, &std_table_css);
> +		init_core_table_attr(&std_table_css, (pstates_num+2),
> +				core_tbl_size.rows, core_tbl_size.cols);
> +		report.add_table(core_data, &std_table_css);
> +		init_cpu_table_attr(&std_table_css, (pstates_num+2),
> +				cpu_tbl_size.rows, cpu_tbl_size.cols);
> +		report.add_table(cpu_data, &std_table_css);
>  	}
> +	report.end_div();
>  }
>  
>  void impl_w_display_cpu_states(int state)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Powertop] [PATCH v3 18/31] report: CPU Frequency html & csv
@ 2013-12-12 19:11 Kristen Carlson Accardi
  0 siblings, 0 replies; 6+ messages in thread
From: Kristen Carlson Accardi @ 2013-12-12 19:11 UTC (permalink / raw)
  To: powertop

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

On Thu, 12 Dec 2013 11:02:19 +0300
Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com> wrote:

> On (12/11/13 11:08), Alexandra Yates wrote:
> > >> On (11/18/13 12:27), Alexandra Yates wrote:
> [..]
> > >>
> > >>
> > >> initialisation:
> > >>
> > >>        report.begin_section(SECTION_CPUFREQ);
> > >>        report.add_header("Processor Frequency Report");
> > >>
> > >>        report.begin_table(TABLE_WIDE);
> > >>
> > >> vs
> > >>
> > >>        /* div attr css_class and css_id */
> > >>        tag_attr div_attr;
> > >>        init_div(&div_attr, "clear_block", "cpufreq");
> > >>
> > >>        /* Set Table attributes, rows, and cols */
> > >>        table_attributes std_table_css;
> > >>        table_size pkg_tbl_size;
> > >>        table_size core_tbl_size;
> > >>        table_size cpu_tbl_size;
> > >>
> > >>
> > >>        /* Set Title attributes */
> > >>        tag_attr title_attr;
> > >>        init_title_attr(&title_attr);
> > >>
> > >>        /* Report add section */
> > >>         report.add_div(&div_attr);
> > >>         report.add_title(&title_attr, __("Processor Frequency Report"));
> > >>
> > >>         /* Set array of data in row Major order */
> > >>        int idx1, idx2, idx3, num_cpus=0;
> > >>         string tmp_str;
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> reporter usage:
> > >>
> > >>
> > >>          report.begin_cell(CELL_CORE_HEADER);
> > >>          report.addf(__("Core %i"), _core->get_number());
> > >>
> > >> vs
> > >>
> > >>          core_data[idx2]="";
> > >>          idx2+=1;
> > >>          sprintf(tmp_num,__("Core %d"),_core->get_number());
> > >>          core_data[idx2]=string(tmp_num);
> > >>          idx2+=1;
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> and
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>          report.begin_cell(CELL_STATE_NAME);
> > >>          report.add(_core->fill_pstate_name(line, buffer));
> > >>          report.begin_cell(CELL_PACKAGE_STATE_VALUE);
> > >>          report.add(_core->fill_pstate_line(line, buffer2));
> > >>
> > >> vs
> > >>
> > >>          tmp_str=string(_core->fill_pstate_name(line, buffer));
> > >>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
> > >>          idx2+=1;
> > >>          tmp_str=string(_core->fill_pstate_line(line, buffer2));
> > >>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
> > >>          idx2+=1;
> > >>
> > >>
> > >> sorry, but the existing one really looks easier to follow and use.
> > >> current
> > >> reporter user
> > >> cares less about reporter internall details.
> > >>
> [..]
> > >
> > > This is matter of opinion.   I found this module the most difficult to
> > > manipulate in general because the complexity of the current code.
> > >
> > > If you look the other examples they are pretty straight forward.
> > >
> > > Thank you,
> > > Alexandra.
> > >
> > 
> > Hi Sergey,
> 
> Hi,
> 
> > I gave more thought at your feedback and you are right to bring attention
> > to  the items here.  As I mentioned before during this iteration of
> > changes I tried to reuse as much code as possible, therefore the current
> > result is not optimal.
> 
> and saying that you pushed NACK-ed patchset to the master. there is a
> `git branch' command for `incremental-make-it-ready-for-master' work.
> my concerns are not about some fancy coding styles or my own opinion.
> new reporter API simply asks for error. and you made off by one error
> using your own `pretty straight forward' API.
> 
> 
[snip]
> 
> I'm not sure that powertop master now even works.
> 
> 	-ss

Hi Sergey,
Pushing Alex's patches, despite the very legitimate issues you raised,
was my call. I did this to force testing on them. Anyone who is looking
for stable powertop should be using a tagged release. As much as I
appreciate that your point of view with regard to having branches for
development (i.e. your next tree, or any other branch) my feeling is
that nobody will test these. Because this UI change is something we
want for 2.6, I decided it was better to push it to master early on and
get the bugs worked out upstream than hang onto it until it was
complete and integrate it too close to March.  I thought sooner rather
than later was better, so we could get as much testing time as possible
on them.

Kristen

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Powertop] [PATCH v3 18/31] report: CPU Frequency html & csv
@ 2013-12-12  8:02 Sergey Senozhatsky
  0 siblings, 0 replies; 6+ messages in thread
From: Sergey Senozhatsky @ 2013-12-12  8:02 UTC (permalink / raw)
  To: powertop

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

On (12/11/13 11:08), Alexandra Yates wrote:
> >> On (11/18/13 12:27), Alexandra Yates wrote:
[..]
> >>
> >>
> >> initialisation:
> >>
> >>        report.begin_section(SECTION_CPUFREQ);
> >>        report.add_header("Processor Frequency Report");
> >>
> >>        report.begin_table(TABLE_WIDE);
> >>
> >> vs
> >>
> >>        /* div attr css_class and css_id */
> >>        tag_attr div_attr;
> >>        init_div(&div_attr, "clear_block", "cpufreq");
> >>
> >>        /* Set Table attributes, rows, and cols */
> >>        table_attributes std_table_css;
> >>        table_size pkg_tbl_size;
> >>        table_size core_tbl_size;
> >>        table_size cpu_tbl_size;
> >>
> >>
> >>        /* Set Title attributes */
> >>        tag_attr title_attr;
> >>        init_title_attr(&title_attr);
> >>
> >>        /* Report add section */
> >>         report.add_div(&div_attr);
> >>         report.add_title(&title_attr, __("Processor Frequency Report"));
> >>
> >>         /* Set array of data in row Major order */
> >>        int idx1, idx2, idx3, num_cpus=0;
> >>         string tmp_str;
> >>
> >>
> >>
> >>
> >>
> >> reporter usage:
> >>
> >>
> >>          report.begin_cell(CELL_CORE_HEADER);
> >>          report.addf(__("Core %i"), _core->get_number());
> >>
> >> vs
> >>
> >>          core_data[idx2]="";
> >>          idx2+=1;
> >>          sprintf(tmp_num,__("Core %d"),_core->get_number());
> >>          core_data[idx2]=string(tmp_num);
> >>          idx2+=1;
> >>
> >>
> >>
> >>
> >>
> >> and
> >>
> >>
> >>
> >>
> >>
> >>          report.begin_cell(CELL_STATE_NAME);
> >>          report.add(_core->fill_pstate_name(line, buffer));
> >>          report.begin_cell(CELL_PACKAGE_STATE_VALUE);
> >>          report.add(_core->fill_pstate_line(line, buffer2));
> >>
> >> vs
> >>
> >>          tmp_str=string(_core->fill_pstate_name(line, buffer));
> >>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
> >>          idx2+=1;
> >>          tmp_str=string(_core->fill_pstate_line(line, buffer2));
> >>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
> >>          idx2+=1;
> >>
> >>
> >> sorry, but the existing one really looks easier to follow and use.
> >> current
> >> reporter user
> >> cares less about reporter internall details.
> >>
[..]
> >
> > This is matter of opinion.   I found this module the most difficult to
> > manipulate in general because the complexity of the current code.
> >
> > If you look the other examples they are pretty straight forward.
> >
> > Thank you,
> > Alexandra.
> >
> 
> Hi Sergey,

Hi,

> I gave more thought at your feedback and you are right to bring attention
> to  the items here.  As I mentioned before during this iteration of
> changes I tried to reuse as much code as possible, therefore the current
> result is not optimal.

and saying that you pushed NACK-ed patchset to the master. there is a
`git branch' command for `incremental-make-it-ready-for-master' work.
my concerns are not about some fancy coding styles or my own opinion.
new reporter API simply asks for error. and you made off by one error
using your own `pretty straight forward' API.


[PATCH 19/31]

+       int show_power, cols, rows, idx;

[..]

+       cols=7;
+       if (show_power)
+               cols=8;

[..]

+       if (show_power)
+               software_data[7]=__("PW Estimate");
+
+       sort(all_power.begin(), all_power.end(), power_cpu_sort);
+       show_power = global_power_valid();

[..]

+               software_data[idx]=string(usage);
+               idx+=1;
+
+               software_data[idx]=string(wakes);
+               idx+=1;
+
+               software_data[idx]=string(gpus);
+               idx+=1;
+
+               software_data[idx]=string(disks);
+               idx+=1;
+
+               software_data[idx]=string(xwakes);
+               idx+=1;
+
+               software_data[idx]=string(name);
+               idx+=1;
+
+               software_data[idx]=string(pretty_print(all_power[i]->description(), descr, 128));
+               idx+=1;
                if (show_power) {
-                       report.begin_cell(CELL_SOFTWARE_POWER);
-                       report.add(power);
+                       software_data[idx]=string(power);
+                       idx+=1;
                }



I'm not sure that powertop master now even works.

	-ss

> However, it is part of my plan to continue changing the UI code to remove
> clutter like the once you pointed.  But this changes will come in the next
> phase of development, because it will be more intrusive. The current
> changes  are incremental so that we have a chance to do testing in between
> the big changes of the UI.
> 
> Thank you again for taking the time to look at my code and your feedback,
> I will make sure to incorporate it as a requirement for phase two.
> 
> Alexandra.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Powertop] [PATCH v3 18/31] report: CPU Frequency html & csv
@ 2013-12-11 19:08 Alexandra Yates
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandra Yates @ 2013-12-11 19:08 UTC (permalink / raw)
  To: powertop

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


>
>> On (11/18/13 12:27), Alexandra Yates wrote:
>>> Enables html and csv report new stlyes for "CPU Frequency" section,
>>> While preserving translation macros.
>>>
>>> Signed-off-by: Alexandra Yates <alexandra.yates(a)linux.intel.com>
>>> ---
>>>  src/cpu/cpu.cpp |  139
>>> ++++++++++++++++++++++++++++++++++++++++---------------
>>>  1 file changed, 101 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
>>> index b8f74a8..c3787e6 100644
>>> --- a/src/cpu/cpu.cpp
>>> +++ b/src/cpu/cpu.cpp
>>> @@ -612,29 +612,81 @@ void report_display_cpu_cstates(void)
>>>
>>>  void report_display_cpu_pstates(void)
>>>  {
>>> -	char buffer[512], buffer2[512];
>>> +	char buffer[512], buffer2[512], tmp_num[50];
>>>  	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++)
>>> +	/* div attr css_class and css_id */
>>> +	tag_attr div_attr;
>>> +	init_div(&div_attr, "clear_block", "cpufreq");
>>> +
>>> +	/* Set Table attributes, rows, and cols */
>>> +	table_attributes std_table_css;
>>> +	table_size pkg_tbl_size;
>>> +	table_size core_tbl_size;
>>> +	table_size cpu_tbl_size;
>>> +
>>> +
>>> +	/* Set Title attributes */
>>> +	tag_attr title_attr;
>>> +	init_title_attr(&title_attr);
>>> +
>>> +	/* Report add section */
>>> +        report.add_div(&div_attr);
>>> +        report.add_title(&title_attr, __("Processor Frequency
>>> Report"));
>>> +
>>> +        /* Set array of data in row Major order */
>>> +	int idx1, idx2, idx3, num_cpus=0;
>>> +        string tmp_str;
>>> +
>>> +	for (i = 0, pstates_num = 0; i < all_cpus.size(); i++) {
>>>  		if (all_cpus[i])
>>>  			pstates_num = std::max<unsigned int>(pstates_num,
>>> -								all_cpus[i]->pstates.size());
>>> -
>>> -	report.begin_section(SECTION_CPUFREQ);
>>> -	report.add_header("Processor Frequency Report");
>>> +				all_cpus[i]->pstates.size());
>>> +	}
>>>
>>>  	for (package = 0; package < system_level.children.size(); package++)
>>> {
>>>  		bool first_core = true;
>>> +		idx1=0;
>>> +		idx2=0;
>>> +		idx3=0;
>>>
>>>  		_package = system_level.children[package];
>>>  		if (!_package)
>>>  			continue;
>>>
>>> -		report.begin_table(TABLE_WIDE);
>>> +		/*  Tables for PKG, CORE, CPU */
>>> +		pkg_tbl_size.cols=2;
>>> +		pkg_tbl_size.rows=pstates_num+2;
>>> +		string pkg_data[pkg_tbl_size.cols * pkg_tbl_size.rows];
>>> +
>>> +		core_tbl_size.cols=2;
>>> +		core_tbl_size.rows=((pstates_num+2) *_package->children.size());
>>> +		string core_data[core_tbl_size.cols * core_tbl_size.rows];
>>> +
>>> +		/* PKG */
>>> +		num_cpus=0;
>>> +		for (core = 0; core < _package->children.size(); core++) {
>>> +			_core = _package->children[core];
>>> +			if (!_core)
>>> +				continue;
>>> +			for (cpu = 0; cpu < _core->children.size(); cpu++) {
>>> +				if (!_cpu)
>>> +				continue;
>>> +				_cpu = _core->children[cpu];
>>> +				num_cpus+=1;
>>> +			}
>>> +		}
>>> +		cpu_tbl_size.cols= num_cpus+1;
>>> +		cpu_tbl_size.rows= (pstates_num+2)* _package->children.size();
>>> +		string cpu_data[cpu_tbl_size.cols * cpu_tbl_size.rows];
>>> +
>>> +		/* Core */
>>>  		for (core = 0; core < _package->children.size(); core++) {
>>> +			cpu_data[idx3]="&nbsp;";
>>> +			idx3+=1;
>>>  			_core = _package->children[core];
>>>  			if (!_core)
>>>  				continue;
>>> @@ -646,75 +698,86 @@ void report_display_cpu_pstates(void)
>>>  				bool first_cpu = true;
>>>
>>>  				if (!_package->has_pstate_level(line))
>>> -					continue;
>>> -
>>> -				report.begin_row();
>>> +				continue;
>>>
>>>  				buffer[0] = 0;
>>>  				buffer2[0] = 0;
>>>  				if (first_core) {
>>>  					if (line == LEVEL_HEADER) {
>>> -						report.begin_cell(CELL_FIRST_PACKAGE_HEADER);
>>> -						report.addf(__("Package %i"), _package->get_number());
>>> +						pkg_data[idx1]=__("Package");
>>> +						idx1+=1;
>>> +						sprintf(tmp_num,"%d",  _package->get_number());
>>> +						pkg_data[idx1]= string(tmp_num);
>>> +						idx1+=1;
>>
>>
>> initialisation:
>>
>>        report.begin_section(SECTION_CPUFREQ);
>>        report.add_header("Processor Frequency Report");
>>
>>        report.begin_table(TABLE_WIDE);
>>
>> vs
>>
>>        /* div attr css_class and css_id */
>>        tag_attr div_attr;
>>        init_div(&div_attr, "clear_block", "cpufreq");
>>
>>        /* Set Table attributes, rows, and cols */
>>        table_attributes std_table_css;
>>        table_size pkg_tbl_size;
>>        table_size core_tbl_size;
>>        table_size cpu_tbl_size;
>>
>>
>>        /* Set Title attributes */
>>        tag_attr title_attr;
>>        init_title_attr(&title_attr);
>>
>>        /* Report add section */
>>         report.add_div(&div_attr);
>>         report.add_title(&title_attr, __("Processor Frequency Report"));
>>
>>         /* Set array of data in row Major order */
>>        int idx1, idx2, idx3, num_cpus=0;
>>         string tmp_str;
>>
>>
>>
>>
>>
>> reporter usage:
>>
>>
>>          report.begin_cell(CELL_CORE_HEADER);
>>          report.addf(__("Core %i"), _core->get_number());
>>
>> vs
>>
>>          core_data[idx2]="";
>>          idx2+=1;
>>          sprintf(tmp_num,__("Core %d"),_core->get_number());
>>          core_data[idx2]=string(tmp_num);
>>          idx2+=1;
>>
>>
>>
>>
>>
>> and
>>
>>
>>
>>
>>
>>          report.begin_cell(CELL_STATE_NAME);
>>          report.add(_core->fill_pstate_name(line, buffer));
>>          report.begin_cell(CELL_PACKAGE_STATE_VALUE);
>>          report.add(_core->fill_pstate_line(line, buffer2));
>>
>> vs
>>
>>          tmp_str=string(_core->fill_pstate_name(line, buffer));
>>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>>          idx2+=1;
>>          tmp_str=string(_core->fill_pstate_line(line, buffer2));
>>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>>          idx2+=1;
>>
>>
>> sorry, but the existing one really looks easier to follow and use.
>> current
>> reporter user
>> cares less about reporter internall details.
>>
>> 	-ss
>>
>>
>>>  					} else {
>>> -						report.begin_cell(CELL_STATE_NAME);
>>> -						report.add(_package->fill_pstate_name(line, buffer));
>>> -						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
>>> -						report.add(_package->fill_pstate_line(line, buffer2));
>>> +						tmp_str=string(_package->fill_pstate_name(line, buffer));
>>> +						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>>> +						idx1+=1;
>>> +						tmp_str=string(_package->fill_pstate_line(line, buffer2));
>>> +						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>>> +						idx1+=1;
>>>  					}
>>> -				} else {
>>> -					report.begin_cell(CELL_EMPTY_PACKAGE_STATE);
>>> -					report.add_empty_cell();
>>>  				}
>>>
>>> -				report.begin_cell(CELL_SEPARATOR);
>>> -				report.add_empty_cell();
>>>
>>>  				if (!_core->can_collapse()) {
>>>  					buffer[0] = 0;
>>>  					buffer2[0] = 0;
>>>  					if (line == LEVEL_HEADER) {
>>> -						report.begin_cell(CELL_CORE_HEADER);
>>> -						report.addf(__("Core %i"), _core->get_number());
>>> +						core_data[idx2]="";
>>> +						idx2+=1;
>>> +						sprintf(tmp_num,__("Core %d"),_core->get_number());
>>> +						core_data[idx2]=string(tmp_num);
>>> +						idx2+=1;
>>>  					} else {
>>> -						report.begin_cell(CELL_STATE_NAME);
>>> -						report.add(_core->fill_pstate_name(line, buffer));
>>> -						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
>>> -						report.add(_core->fill_pstate_line(line, buffer2));
>>> +						tmp_str=string(_core->fill_pstate_name(line, buffer));
>>> +						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>>> +						idx2+=1;
>>> +						tmp_str=string(_core->fill_pstate_line(line, buffer2));
>>> +						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>>> +						idx2+=1;
>>>  					}
>>>  				}
>>>
>>> -				report.begin_cell(CELL_SEPARATOR);
>>> -				report.add_empty_cell();
>>> -
>>> +				/* CPU */
>>>  				for (cpu = 0; cpu < _core->children.size(); cpu++) {
>>>  					buffer[0] = 0;
>>>  					_cpu = _core->children[cpu];
>>>  					if (!_cpu)
>>>  						continue;
>>>
>>> -					report.set_cpu_number(cpu);
>>>  					if (line == LEVEL_HEADER) {
>>> -						report.begin_cell(CELL_CPU_PSTATE_HEADER);
>>> -						report.addf(__("CPU %i"), _cpu->get_number());
>>> +						sprintf(tmp_num,__("CPU %d"),_cpu->get_number());
>>> +						cpu_data[idx3] = string(tmp_num);
>>> +						idx3+=1;
>>>  						continue;
>>>  					}
>>>
>>>  					if (first_cpu) {
>>> -						report.begin_cell(CELL_STATE_NAME);
>>> -						report.add(_cpu->fill_pstate_name(line, buffer));
>>> +						tmp_str=string(_cpu->fill_pstate_name(line, buffer));
>>> +						cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>>> +						idx3+=1;
>>>  						first_cpu = false;
>>>  					}
>>>
>>>  					buffer[0] = 0;
>>> -					report.begin_cell(CELL_CPU_STATE_VALUE);
>>> -					report.add(_cpu->fill_pstate_line(line, buffer));
>>> +					tmp_str=string(_cpu->fill_pstate_line(line, buffer));
>>> +					cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>>> +					idx3+=1;
>>>  				}
>>>  			}
>>> -
>>>  			first_core = false;
>>>  		}
>>> +		init_pkg_table_attr(&std_table_css, pkg_tbl_size.rows,
>>> pkg_tbl_size.cols);
>>> +		report.add_table(pkg_data, &std_table_css);
>>> +		init_core_table_attr(&std_table_css, (pstates_num+2),
>>> +				core_tbl_size.rows, core_tbl_size.cols);
>>> +		report.add_table(core_data, &std_table_css);
>>> +		init_cpu_table_attr(&std_table_css, (pstates_num+2),
>>> +				cpu_tbl_size.rows, cpu_tbl_size.cols);
>>> +		report.add_table(cpu_data, &std_table_css);
>>>  	}
>>> +	report.end_div();
>>>  }
>>>
>>>  void impl_w_display_cpu_states(int state)
>>> --
>>> 1.7.9.5
>>>
>>> _______________________________________________
>>> PowerTop mailing list
>>> PowerTop(a)lists.01.org
>>> https://lists.01.org/mailman/listinfo/powertop
>>>
>>
>
> This is matter of opinion.   I found this module the most difficult to
> manipulate in general because the complexity of the current code.
>
> If you look the other examples they are pretty straight forward.
>
> Thank you,
> Alexandra.
>

Hi Sergey,

I gave more thought at your feedback and you are right to bring attention
to  the items here.  As I mentioned before during this iteration of
changes I tried to reuse as much code as possible, therefore the current
result is not optimal.

However, it is part of my plan to continue changing the UI code to remove
clutter like the once you pointed.  But this changes will come in the next
phase of development, because it will be more intrusive. The current
changes  are incremental so that we have a chance to do testing in between
the big changes of the UI.

Thank you again for taking the time to look at my code and your feedback,
I will make sure to incorporate it as a requirement for phase two.

Alexandra.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Powertop] [PATCH v3 18/31] report: CPU Frequency html & csv
@ 2013-12-11  0:36 Alexandra Yates
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandra Yates @ 2013-12-11  0:36 UTC (permalink / raw)
  To: powertop

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


> On (11/18/13 12:27), Alexandra Yates wrote:
>> Enables html and csv report new stlyes for "CPU Frequency" section,
>> While preserving translation macros.
>>
>> Signed-off-by: Alexandra Yates <alexandra.yates(a)linux.intel.com>
>> ---
>>  src/cpu/cpu.cpp |  139
>> ++++++++++++++++++++++++++++++++++++++++---------------
>>  1 file changed, 101 insertions(+), 38 deletions(-)
>>
>> diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
>> index b8f74a8..c3787e6 100644
>> --- a/src/cpu/cpu.cpp
>> +++ b/src/cpu/cpu.cpp
>> @@ -612,29 +612,81 @@ void report_display_cpu_cstates(void)
>>
>>  void report_display_cpu_pstates(void)
>>  {
>> -	char buffer[512], buffer2[512];
>> +	char buffer[512], buffer2[512], tmp_num[50];
>>  	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++)
>> +	/* div attr css_class and css_id */
>> +	tag_attr div_attr;
>> +	init_div(&div_attr, "clear_block", "cpufreq");
>> +
>> +	/* Set Table attributes, rows, and cols */
>> +	table_attributes std_table_css;
>> +	table_size pkg_tbl_size;
>> +	table_size core_tbl_size;
>> +	table_size cpu_tbl_size;
>> +
>> +
>> +	/* Set Title attributes */
>> +	tag_attr title_attr;
>> +	init_title_attr(&title_attr);
>> +
>> +	/* Report add section */
>> +        report.add_div(&div_attr);
>> +        report.add_title(&title_attr, __("Processor Frequency
>> Report"));
>> +
>> +        /* Set array of data in row Major order */
>> +	int idx1, idx2, idx3, num_cpus=0;
>> +        string tmp_str;
>> +
>> +	for (i = 0, pstates_num = 0; i < all_cpus.size(); i++) {
>>  		if (all_cpus[i])
>>  			pstates_num = std::max<unsigned int>(pstates_num,
>> -								all_cpus[i]->pstates.size());
>> -
>> -	report.begin_section(SECTION_CPUFREQ);
>> -	report.add_header("Processor Frequency Report");
>> +				all_cpus[i]->pstates.size());
>> +	}
>>
>>  	for (package = 0; package < system_level.children.size(); package++) {
>>  		bool first_core = true;
>> +		idx1=0;
>> +		idx2=0;
>> +		idx3=0;
>>
>>  		_package = system_level.children[package];
>>  		if (!_package)
>>  			continue;
>>
>> -		report.begin_table(TABLE_WIDE);
>> +		/*  Tables for PKG, CORE, CPU */
>> +		pkg_tbl_size.cols=2;
>> +		pkg_tbl_size.rows=pstates_num+2;
>> +		string pkg_data[pkg_tbl_size.cols * pkg_tbl_size.rows];
>> +
>> +		core_tbl_size.cols=2;
>> +		core_tbl_size.rows=((pstates_num+2) *_package->children.size());
>> +		string core_data[core_tbl_size.cols * core_tbl_size.rows];
>> +
>> +		/* PKG */
>> +		num_cpus=0;
>> +		for (core = 0; core < _package->children.size(); core++) {
>> +			_core = _package->children[core];
>> +			if (!_core)
>> +				continue;
>> +			for (cpu = 0; cpu < _core->children.size(); cpu++) {
>> +				if (!_cpu)
>> +				continue;
>> +				_cpu = _core->children[cpu];
>> +				num_cpus+=1;
>> +			}
>> +		}
>> +		cpu_tbl_size.cols= num_cpus+1;
>> +		cpu_tbl_size.rows= (pstates_num+2)* _package->children.size();
>> +		string cpu_data[cpu_tbl_size.cols * cpu_tbl_size.rows];
>> +
>> +		/* Core */
>>  		for (core = 0; core < _package->children.size(); core++) {
>> +			cpu_data[idx3]="&nbsp;";
>> +			idx3+=1;
>>  			_core = _package->children[core];
>>  			if (!_core)
>>  				continue;
>> @@ -646,75 +698,86 @@ void report_display_cpu_pstates(void)
>>  				bool first_cpu = true;
>>
>>  				if (!_package->has_pstate_level(line))
>> -					continue;
>> -
>> -				report.begin_row();
>> +				continue;
>>
>>  				buffer[0] = 0;
>>  				buffer2[0] = 0;
>>  				if (first_core) {
>>  					if (line == LEVEL_HEADER) {
>> -						report.begin_cell(CELL_FIRST_PACKAGE_HEADER);
>> -						report.addf(__("Package %i"), _package->get_number());
>> +						pkg_data[idx1]=__("Package");
>> +						idx1+=1;
>> +						sprintf(tmp_num,"%d",  _package->get_number());
>> +						pkg_data[idx1]= string(tmp_num);
>> +						idx1+=1;
>
>
> initialisation:
>
>        report.begin_section(SECTION_CPUFREQ);
>        report.add_header("Processor Frequency Report");
>
>        report.begin_table(TABLE_WIDE);
>
> vs
>
>        /* div attr css_class and css_id */
>        tag_attr div_attr;
>        init_div(&div_attr, "clear_block", "cpufreq");
>
>        /* Set Table attributes, rows, and cols */
>        table_attributes std_table_css;
>        table_size pkg_tbl_size;
>        table_size core_tbl_size;
>        table_size cpu_tbl_size;
>
>
>        /* Set Title attributes */
>        tag_attr title_attr;
>        init_title_attr(&title_attr);
>
>        /* Report add section */
>         report.add_div(&div_attr);
>         report.add_title(&title_attr, __("Processor Frequency Report"));
>
>         /* Set array of data in row Major order */
>        int idx1, idx2, idx3, num_cpus=0;
>         string tmp_str;
>
>
>
>
>
> reporter usage:
>
>
>          report.begin_cell(CELL_CORE_HEADER);
>          report.addf(__("Core %i"), _core->get_number());
>
> vs
>
>          core_data[idx2]="";
>          idx2+=1;
>          sprintf(tmp_num,__("Core %d"),_core->get_number());
>          core_data[idx2]=string(tmp_num);
>          idx2+=1;
>
>
>
>
>
> and
>
>
>
>
>
>          report.begin_cell(CELL_STATE_NAME);
>          report.add(_core->fill_pstate_name(line, buffer));
>          report.begin_cell(CELL_PACKAGE_STATE_VALUE);
>          report.add(_core->fill_pstate_line(line, buffer2));
>
> vs
>
>          tmp_str=string(_core->fill_pstate_name(line, buffer));
>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>          idx2+=1;
>          tmp_str=string(_core->fill_pstate_line(line, buffer2));
>          core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>          idx2+=1;
>
>
> sorry, but the existing one really looks easier to follow and use. current
> reporter user
> cares less about reporter internall details.
>
> 	-ss
>
>
>>  					} else {
>> -						report.begin_cell(CELL_STATE_NAME);
>> -						report.add(_package->fill_pstate_name(line, buffer));
>> -						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
>> -						report.add(_package->fill_pstate_line(line, buffer2));
>> +						tmp_str=string(_package->fill_pstate_name(line, buffer));
>> +						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>> +						idx1+=1;
>> +						tmp_str=string(_package->fill_pstate_line(line, buffer2));
>> +						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>> +						idx1+=1;
>>  					}
>> -				} else {
>> -					report.begin_cell(CELL_EMPTY_PACKAGE_STATE);
>> -					report.add_empty_cell();
>>  				}
>>
>> -				report.begin_cell(CELL_SEPARATOR);
>> -				report.add_empty_cell();
>>
>>  				if (!_core->can_collapse()) {
>>  					buffer[0] = 0;
>>  					buffer2[0] = 0;
>>  					if (line == LEVEL_HEADER) {
>> -						report.begin_cell(CELL_CORE_HEADER);
>> -						report.addf(__("Core %i"), _core->get_number());
>> +						core_data[idx2]="";
>> +						idx2+=1;
>> +						sprintf(tmp_num,__("Core %d"),_core->get_number());
>> +						core_data[idx2]=string(tmp_num);
>> +						idx2+=1;
>>  					} else {
>> -						report.begin_cell(CELL_STATE_NAME);
>> -						report.add(_core->fill_pstate_name(line, buffer));
>> -						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
>> -						report.add(_core->fill_pstate_line(line, buffer2));
>> +						tmp_str=string(_core->fill_pstate_name(line, buffer));
>> +						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>> +						idx2+=1;
>> +						tmp_str=string(_core->fill_pstate_line(line, buffer2));
>> +						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
>> +						idx2+=1;
>>  					}
>>  				}
>>
>> -				report.begin_cell(CELL_SEPARATOR);
>> -				report.add_empty_cell();
>> -
>> +				/* CPU */
>>  				for (cpu = 0; cpu < _core->children.size(); cpu++) {
>>  					buffer[0] = 0;
>>  					_cpu = _core->children[cpu];
>>  					if (!_cpu)
>>  						continue;
>>
>> -					report.set_cpu_number(cpu);
>>  					if (line == LEVEL_HEADER) {
>> -						report.begin_cell(CELL_CPU_PSTATE_HEADER);
>> -						report.addf(__("CPU %i"), _cpu->get_number());
>> +						sprintf(tmp_num,__("CPU %d"),_cpu->get_number());
>> +						cpu_data[idx3] = string(tmp_num);
>> +						idx3+=1;
>>  						continue;
>>  					}
>>
>>  					if (first_cpu) {
>> -						report.begin_cell(CELL_STATE_NAME);
>> -						report.add(_cpu->fill_pstate_name(line, buffer));
>> +						tmp_str=string(_cpu->fill_pstate_name(line, buffer));
>> +						cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>> +						idx3+=1;
>>  						first_cpu = false;
>>  					}
>>
>>  					buffer[0] = 0;
>> -					report.begin_cell(CELL_CPU_STATE_VALUE);
>> -					report.add(_cpu->fill_pstate_line(line, buffer));
>> +					tmp_str=string(_cpu->fill_pstate_line(line, buffer));
>> +					cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
>> +					idx3+=1;
>>  				}
>>  			}
>> -
>>  			first_core = false;
>>  		}
>> +		init_pkg_table_attr(&std_table_css, pkg_tbl_size.rows,
>> pkg_tbl_size.cols);
>> +		report.add_table(pkg_data, &std_table_css);
>> +		init_core_table_attr(&std_table_css, (pstates_num+2),
>> +				core_tbl_size.rows, core_tbl_size.cols);
>> +		report.add_table(core_data, &std_table_css);
>> +		init_cpu_table_attr(&std_table_css, (pstates_num+2),
>> +				cpu_tbl_size.rows, cpu_tbl_size.cols);
>> +		report.add_table(cpu_data, &std_table_css);
>>  	}
>> +	report.end_div();
>>  }
>>
>>  void impl_w_display_cpu_states(int state)
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> PowerTop mailing list
>> PowerTop(a)lists.01.org
>> https://lists.01.org/mailman/listinfo/powertop
>>
>

This is matter of opinion.   I found this module the most difficult to
manipulate in general because the complexity of the current code.

If you look the other examples they are pretty straight forward.

Thank you,
Alexandra.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Powertop] [PATCH v3 18/31] report: CPU Frequency html & csv
@ 2013-11-18 20:27 Alexandra Yates
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandra Yates @ 2013-11-18 20:27 UTC (permalink / raw)
  To: powertop

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

Enables html and csv report new stlyes for "CPU Frequency" section,
While preserving translation macros.

Signed-off-by: Alexandra Yates <alexandra.yates(a)linux.intel.com>
---
 src/cpu/cpu.cpp |  139 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 101 insertions(+), 38 deletions(-)

diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
index b8f74a8..c3787e6 100644
--- a/src/cpu/cpu.cpp
+++ b/src/cpu/cpu.cpp
@@ -612,29 +612,81 @@ void report_display_cpu_cstates(void)
 
 void report_display_cpu_pstates(void)
 {
-	char buffer[512], buffer2[512];
+	char buffer[512], buffer2[512], tmp_num[50];
 	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++)
+	/* div attr css_class and css_id */
+	tag_attr div_attr;
+	init_div(&div_attr, "clear_block", "cpufreq");
+
+	/* Set Table attributes, rows, and cols */
+	table_attributes std_table_css;
+	table_size pkg_tbl_size;
+	table_size core_tbl_size;
+	table_size cpu_tbl_size;
+
+
+	/* Set Title attributes */
+	tag_attr title_attr;
+	init_title_attr(&title_attr);
+
+	/* Report add section */
+        report.add_div(&div_attr);
+        report.add_title(&title_attr, __("Processor Frequency Report"));
+
+        /* Set array of data in row Major order */
+	int idx1, idx2, idx3, num_cpus=0;
+        string tmp_str;
+
+	for (i = 0, pstates_num = 0; i < all_cpus.size(); i++) {
 		if (all_cpus[i])
 			pstates_num = std::max<unsigned int>(pstates_num,
-								all_cpus[i]->pstates.size());
-
-	report.begin_section(SECTION_CPUFREQ);
-	report.add_header("Processor Frequency Report");
+				all_cpus[i]->pstates.size());
+	}
 
 	for (package = 0; package < system_level.children.size(); package++) {
 		bool first_core = true;
+		idx1=0;
+		idx2=0;
+		idx3=0;
 
 		_package = system_level.children[package];
 		if (!_package)
 			continue;
 
-		report.begin_table(TABLE_WIDE);
+		/*  Tables for PKG, CORE, CPU */
+		pkg_tbl_size.cols=2;
+		pkg_tbl_size.rows=pstates_num+2;
+		string pkg_data[pkg_tbl_size.cols * pkg_tbl_size.rows];
+
+		core_tbl_size.cols=2;
+		core_tbl_size.rows=((pstates_num+2) *_package->children.size());
+		string core_data[core_tbl_size.cols * core_tbl_size.rows];
+
+		/* PKG */
+		num_cpus=0;
+		for (core = 0; core < _package->children.size(); core++) {
+			_core = _package->children[core];
+			if (!_core)
+				continue;
+			for (cpu = 0; cpu < _core->children.size(); cpu++) {
+				if (!_cpu)
+				continue;
+				_cpu = _core->children[cpu];
+				num_cpus+=1;
+			}
+		}
+		cpu_tbl_size.cols= num_cpus+1;
+		cpu_tbl_size.rows= (pstates_num+2)* _package->children.size();
+		string cpu_data[cpu_tbl_size.cols * cpu_tbl_size.rows];
+
+		/* Core */
 		for (core = 0; core < _package->children.size(); core++) {
+			cpu_data[idx3]="&nbsp;";
+			idx3+=1;
 			_core = _package->children[core];
 			if (!_core)
 				continue;
@@ -646,75 +698,86 @@ void report_display_cpu_pstates(void)
 				bool first_cpu = true;
 
 				if (!_package->has_pstate_level(line))
-					continue;
-
-				report.begin_row();
+				continue;
 
 				buffer[0] = 0;
 				buffer2[0] = 0;
 				if (first_core) {
 					if (line == LEVEL_HEADER) {
-						report.begin_cell(CELL_FIRST_PACKAGE_HEADER);
-						report.addf(__("Package %i"), _package->get_number());
+						pkg_data[idx1]=__("Package");
+						idx1+=1;
+						sprintf(tmp_num,"%d",  _package->get_number());
+						pkg_data[idx1]= string(tmp_num);
+						idx1+=1;
 					} else {
-						report.begin_cell(CELL_STATE_NAME);
-						report.add(_package->fill_pstate_name(line, buffer));
-						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
-						report.add(_package->fill_pstate_line(line, buffer2));
+						tmp_str=string(_package->fill_pstate_name(line, buffer));
+						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
+						idx1+=1;
+						tmp_str=string(_package->fill_pstate_line(line, buffer2));
+						pkg_data[idx1]=(tmp_str=="" ? "&nbsp;" : tmp_str);
+						idx1+=1;
 					}
-				} else {
-					report.begin_cell(CELL_EMPTY_PACKAGE_STATE);
-					report.add_empty_cell();
 				}
 
-				report.begin_cell(CELL_SEPARATOR);
-				report.add_empty_cell();
 
 				if (!_core->can_collapse()) {
 					buffer[0] = 0;
 					buffer2[0] = 0;
 					if (line == LEVEL_HEADER) {
-						report.begin_cell(CELL_CORE_HEADER);
-						report.addf(__("Core %i"), _core->get_number());
+						core_data[idx2]="";
+						idx2+=1;
+						sprintf(tmp_num,__("Core %d"),_core->get_number());
+						core_data[idx2]=string(tmp_num);
+						idx2+=1;
 					} else {
-						report.begin_cell(CELL_STATE_NAME);
-						report.add(_core->fill_pstate_name(line, buffer));
-						report.begin_cell(CELL_PACKAGE_STATE_VALUE);
-						report.add(_core->fill_pstate_line(line, buffer2));
+						tmp_str=string(_core->fill_pstate_name(line, buffer));
+						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
+						idx2+=1;
+						tmp_str=string(_core->fill_pstate_line(line, buffer2));
+						core_data[idx2]= (tmp_str=="" ? "&nbsp;" : tmp_str);
+						idx2+=1;
 					}
 				}
 
-				report.begin_cell(CELL_SEPARATOR);
-				report.add_empty_cell();
-
+				/* CPU */
 				for (cpu = 0; cpu < _core->children.size(); cpu++) {
 					buffer[0] = 0;
 					_cpu = _core->children[cpu];
 					if (!_cpu)
 						continue;
 
-					report.set_cpu_number(cpu);
 					if (line == LEVEL_HEADER) {
-						report.begin_cell(CELL_CPU_PSTATE_HEADER);
-						report.addf(__("CPU %i"), _cpu->get_number());
+						sprintf(tmp_num,__("CPU %d"),_cpu->get_number());
+						cpu_data[idx3] = string(tmp_num);
+						idx3+=1;
 						continue;
 					}
 
 					if (first_cpu) {
-						report.begin_cell(CELL_STATE_NAME);
-						report.add(_cpu->fill_pstate_name(line, buffer));
+						tmp_str=string(_cpu->fill_pstate_name(line, buffer));
+						cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
+						idx3+=1;
 						first_cpu = false;
 					}
 
 					buffer[0] = 0;
-					report.begin_cell(CELL_CPU_STATE_VALUE);
-					report.add(_cpu->fill_pstate_line(line, buffer));
+					tmp_str=string(_cpu->fill_pstate_line(line, buffer));
+					cpu_data[idx3]=(tmp_str=="" ? "&nbsp;" : tmp_str);
+					idx3+=1;
 				}
 			}
-
 			first_core = false;
 		}
+		init_pkg_table_attr(&std_table_css, pkg_tbl_size.rows, pkg_tbl_size.cols);
+		report.add_table(pkg_data, &std_table_css);
+		init_core_table_attr(&std_table_css, (pstates_num+2),
+				core_tbl_size.rows, core_tbl_size.cols);
+		report.add_table(core_data, &std_table_css);
+		init_cpu_table_attr(&std_table_css, (pstates_num+2),
+				cpu_tbl_size.rows, cpu_tbl_size.cols);
+		report.add_table(cpu_data, &std_table_css);
 	}
+	report.end_div();
 }
 
 void impl_w_display_cpu_states(int state)
-- 
1.7.9.5


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

end of thread, other threads:[~2013-12-12 19:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-10 20:12 [Powertop] [PATCH v3 18/31] report: CPU Frequency html & csv Sergey Senozhatsky
  -- strict thread matches above, loose matches on Subject: below --
2013-12-12 19:11 Kristen Carlson Accardi
2013-12-12  8:02 Sergey Senozhatsky
2013-12-11 19:08 Alexandra Yates
2013-12-11  0:36 Alexandra Yates
2013-11-18 20:27 Alexandra Yates

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.