Re: [Powertop] [PATCH 4/4] Add support for Intel GPU statistics

[Arjan van de Ven <arjan at linux.intel.com>]

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

>>
>> +	file.open("/sys/class/drm/card0/power/rc6_residency_ms",  ios::in);
>> +
>> +	if (file) {
>> +		handle_i965_gpu();
>> +		file.close();
>> +	}
>> +
> 
> side note: just wonder how much C++ stream with its heavy buffering, etc. is slower
> than stat(). I'll review and if it makes sense will probably prepare simple stat()
> wrapper to lib.cpp

better to use access() than stat.

both have the fun of getting a bunch of low level system headers into C++
I'm sure that'll work most of the time, but for something as non-time critical as this
I wonder how badly that is inviting trouble.




>> +	
>> +	if (line_nr == 0)
>> +		d = 100.0 - ratio * (rc6_after + rc6p_after + rc6pp_after - rc6_before - rc6p_before - rc6pp_before);
>> +	if (line_nr == 1)
>> +		d = ratio * (rc6_after - rc6_before);
>> +	if (line_nr == 2)
>> +		d = ratio * (rc6p_after - rc6p_before);
>> +	if (line_nr == 3)
>> +		d = ratio * (rc6pp_after - rc6pp_before);
>> +	if (line_nr >= 4 || line_nr < 0)
>> +		return buffer;
>> +
> 
> small side note /* someone will do it anyway :-) */:
> how about
> 
> 	if (line_nr == 0)
> 		d = 100.0 - ratio * (rc6_after + rc6p_after + rc6pp_after - rc6_before - rc6p_before - rc6pp_before);
> 	else if (line_nr == 1)
> 		d = ratio * (rc6_after - rc6_before);
> 	else if (line_nr == 2)
> 		d = ratio * (rc6p_after - rc6p_before);
> 	else if (line_nr == 3)
> 		d = ratio * (rc6pp_after - rc6pp_before);
> 	else if (line_nr >= 4 || line_nr < 0)
> 		return buffer;

well last time I looked at the disassembly for such a case, this generated worse code.
I'll admit that that was about 2 gcc versions ago though.