Re: [PATCH] Make PowerTOP generic, by removing hard coded values

Re: [PATCH] Make PowerTOP generic, by removing hard coded values

[Thomas Renninger]

On Tuesday 24 August 2010 12:58:41 Amit Arora wrote:
> Hello Auke,
> 
> Could you please look at this patch and provide your feedback ? 
> Thanks!
While this is probably needed for your purposes for a shortterm 
solution, I hope powertop can link against cpufrequtils library in the 
future to fetch general cpu freq info.
cpufrequtils is well maintained by one of the cpufreq kernel authors
and should be available on every distro for quite some time.

When I looked at this, I realized that this is perfect for using 
cpufreqlib and I wrote a patch making use of it.
I'll send 3 patches.
The first 2, I'd like to see getting applied, the third isn't that much
of use, but also an improvement.

There also was the discussion about cleaner sysfs access for pci stuff
(instead of lspci external binary executions and similar).
It would also be nice to see libpci getting used instead of 
reimplementing its functionality.

Amit: Your problem seem not to be the 16 static freqs, oldfreqs, delta
arrays, but that only 6 frequencies max are displayed?:

char cpufreqstrings[6][80];
...
if (maxfreq>4)
	maxfreq=4;
which should limit the frequency information string to 6 lines.
This you should have to touch?
On the other hand side there already is sorting for most relevant/mostly
used frequencies and only these are cut out. So if you have 16 (or do 
you really have more?) frequencies you should see the 6 mostly used 
ones, which sounds sane -> display size is limited.

Thanks,

    Thomas

> Regards,
> Amit Arora
> 
> On Fri, Aug 13, 2010 at 12:55 PM, Amit Arora <amit.arora@linaro.org>
> wrote:
> > Background:
> > Currently, PowerTOP has hard coded values for the number of C and P
> > states supported. Hence it doesn't work well on systems which may
> > have more states than these hard coded ones. This is specially true
> > for some of the ARM SoCs which may have as many as 8 to 9 C-states.
> >
> > Suggested fix:
> > This patch defines MAX values of C and P states for allocating
> > enough memory for data structures statically. But, uses actual
> > values of the MAX C and P states for display (using, maxcstate and
> > maxpstate variables).
...

Re: [PATCH] Make PowerTOP generic, by removing hard coded values

[Auke Kok]

On 08/25/2010 07:20 AM, Thomas Renninger wrote:
> It would also be nice to see libpci getting used instead of
> reimplementing its functionality.

agreed, this might be the elegant way to solve that problem.

Auke

Re: [PATCH] Make PowerTOP generic, by removing hard coded values

[Amit Arora]

Hello Thomas,

On Wed, Aug 25, 2010 at 7:50 PM, Thomas Renninger <trenn@suse.de> wrote:
> Amit: Your problem seem not to be the 16 static freqs, oldfreqs, delta
> arrays, but that only 6 frequencies max are displayed?:
>
> char cpufreqstrings[6][80];
> ...
> if (maxfreq>4)
>        maxfreq=4;
> which should limit the frequency information string to 6 lines.
> This you should have to touch?

I had seen the above logic. My aim is not to display all the P states.
Am fine with top 4 (after sorting), how it currently does, and thus
didn't suggest to change this in my patch.

Though in some of the ARM boards the number of P states can be less
than 4. Hence the display won't be nice for such boards.

But, actually its the cpuidle states (C states), which bothers me more
than the P states. The problem is that on some of the ARM boards the
number of C states can be as many as 9 !  And different ARM SoC have
different number of these C and P states. So, its not practical to
have them hardcoded for each board!

Hence, we need a way to dynamically find out the number of C and P
states, and display accordingly. And the same code should ideally work
on x86, ARM and any other architecture that may want to use powertop.

> On the other hand side there already is sorting for most relevant/mostly
> used frequencies and only these are cut out. So if you have 16 (or do
> you really have more?) frequencies you should see the 6 mostly used
> ones, which sounds sane -> display size is limited.

As mentioned above, this is perfectly fine.

The goal here is to have the same code function on x86 and various ARM
SoCs - which have different levels of these C and P states.


Thanks!
Regards,
Amit Arora