Re: [Powertop] [PATCH] cpu: allow for more package than core cstates

[Sergey Senozhatsky <sergey.senozhatsky at gmail.com>]

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

On (04/08/13 16:35), Kristen Carlson Accardi wrote:
> Date: Mon,  8 Apr 2013 16:35:41 -0700
> From: Kristen Carlson Accardi <kristen(a)linux.intel.com>
> To: powertop(a)lists.01.org
> Subject: [Powertop] [PATCH] cpu: allow for more package than core cstates
> X-Mailer: git-send-email 1.7.11.7
> 
> Change reporting code to walk package, then core, the cpu to determine max
> number of cstate vs. just walking the logical cpus.
> 
> change display function to obtain max cstate num the same way.
> ---
>  src/cpu/cpu.cpp | 68 ++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 55 insertions(+), 13 deletions(-)
>

looks good to me.

	-ss

 
> diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
> index afe6a8c..9e075bd 100644
> --- a/src/cpu/cpu.cpp
> +++ b/src/cpu/cpu.cpp
> @@ -402,20 +402,64 @@ static const char * fill_state_line(class abstract_cpu *acpu, int state, int lin
>  	return "-EINVAL";
>  }
>  
> +static int get_cstates_num(void)
> +{
> +	unsigned int package, core, cpu;
> +	class abstract_cpu *_package, * _core, * _cpu;
> +	unsigned int i;
> +	int cstates_num;
> +
> +	for (package = 0, cstates_num = 0;
> +			package < system_level.children.size(); package++) {
> +		_package = system_level.children[package];
> +		if (_package == NULL)
> +			continue;
> +
> +		/* walk package cstates and get largest cstates number */
> +		for (i = 0; i < _package->cstates.size(); i++)
> +			cstates_num = std::max(cstates_num,
> +						(_package->cstates[i])->line_level);
> +
> +		/*
> +		 * for each core in this package, walk core cstates and get
> +		 * largest cstates number
> +		 */
> +		for (core = 0; core < _package->children.size(); core++) {
> +			_core = _package->children[core];
> +			if (_core == NULL)
> +				continue;
> +
> +			for (i = 0; i <  _core->cstates.size(); i++)
> +				cstates_num = std::max(cstates_num,
> +						(_core->cstates[i])->line_level);
> +
> +			/*
> +			 * for each core, walk the logical cpus in case
> +			 * there is are more linux cstates than hw cstates
> +			 */
> +			 for (cpu = 0; cpu < _core->children.size(); cpu++) {
> +				_cpu = _core->children[cpu];
> +				if (_cpu == NULL)
> +					continue;
> +
> +				for (i = 0; i < _cpu->cstates.size(); i++)
> +					cstates_num = std::max(cstates_num,
> +						(_cpu->cstates[i])->line_level);
> +			}
> +		}
> +	}
> +
> +	return cstates_num;
> +}
> +
>  void report_display_cpu_cstates(void)
>  {
>  	char buffer[512], buffer2[512];
>  	unsigned int package, core, cpu;
>  	int line, cstates_num;
>  	class abstract_cpu *_package, * _core, * _cpu;
> -	unsigned int i, j;
>  
> -	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++)
> -				cstates_num = std::max(cstates_num,
> -						(all_cpus[i]->cstates[j])->line_level);
> -	}
> +	cstates_num = get_cstates_num();
>  
>  	report.begin_section(SECTION_CPUIDLE);
>  	report.add_header("Processor Idle state report");
> @@ -644,17 +688,15 @@ void impl_w_display_cpu_states(int state)
>  	int line, loop, cstates_num, pstates_num;
>  	class abstract_cpu *_package, * _core, * _cpu;
>  	int ctr = 0;
> -	unsigned int i, j;
> +	unsigned int i;
>  
> -	for (i = 0, cstates_num = 0, pstates_num = 0; i < all_cpus.size(); i++) {
> +	cstates_num = get_cstates_num();
> +
> +	for (i = 0, pstates_num = 0; i < all_cpus.size(); i++) {
>  		if (!all_cpus[i])
>  			continue;
>  
>  		pstates_num = std::max<int>(pstates_num, all_cpus[i]->pstates.size());
> -
> -		for (j = 0; j < all_cpus[i]->cstates.size(); j++)
> -			cstates_num = std::max(cstates_num,
> -						(all_cpus[i]->cstates[j])->line_level);
>  	}
>  
>  	if (state == PSTATE) {
> -- 
> 1.7.11.7
> 
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
>