[RFC 0/1] ARM: print MHz in /proc/cpuinfo

[RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Jon Mason]

Many users (and some applications) are expecting the CPU clock speed to
be output in /proc/cpuinfo (as is done in x86, avr32, c6x, tile, parisc,
ia64, and xtensa).  This can be trivially added by simply querying the
clock described in the CPU node of the device tree.  It appears that
many of the DTSI files in arch/arm/boot/dts already have this defined.
So, this will add this desired functionality for many boards with
already existing information.  For those that do not have this defined,
it will simply not output the string in question (thus keeping
everything the same as before).

The output was modeled after x86 (based on number of significant digits
and location in the output), but is similar to other architectures.  For
example, the output on my local board looks like:

# cat /proc/cpuinfo 
processor	: 0
model name	: ARMv7 Processor rev 0 (v7l)
cpu MHz		: 1200.000
BogoMIPS	: 1200.00
Features	: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpd32 
CPU implementer	: 0x41
CPU architecture: 7
CPU variant	: 0x3
CPU part	: 0xc09
CPU revision	: 0

processor	: 1
model name	: ARMv7 Processor rev 0 (v7l)
cpu MHz		: 1200.000
BogoMIPS	: 1200.00
Features	: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpd32 
CPU implementer	: 0x41
CPU architecture: 7
CPU variant	: 0x3
CPU part	: 0xc09
CPU revision	: 0

Hardware	: Broadcom Northstar Plus SoC
Revision	: 0000
Serial		: 0000000000000000

Thanks,
Jon



Jon Mason (1):
  ARM: print MHz in /proc/cpuinfo

 arch/arm/kernel/setup.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

-- 
1.9.1

[RFC 1/1] ARM: print MHz in /proc/cpuinfo

[Jon Mason]

Query the CPU core clock in the device tree to determine the core clock
speed.  Output this clock rate in /proc/cpuinfo to match the output
from other architectures.  The output is intentionally patterned after
the x86 output, to match existing (and possibly expected) convention.

If any errors are encountered in querying the clock (or the speed is
erroneously zero), nothing will be printed out.  Thus any existing
devices that do not have CPU clocks defined in the device tree will
work as before.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 arch/arm/kernel/setup.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 7b53500..0c3e25a 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -33,6 +33,7 @@
 #include <linux/compiler.h>
 #include <linux/sort.h>
 #include <linux/psci.h>
+#include <linux/clk.h>
 
 #include <asm/unified.h>
 #include <asm/cp15.h>
@@ -1178,10 +1179,32 @@ static const char *hwcap2_str[] = {
 	NULL
 };
 
+static unsigned long cpu_freq(unsigned int core)
+{
+	struct device_node *np;
+	struct clk *c;
+	unsigned long rate = 0;
+
+	np = of_get_cpu_node(core, NULL);
+	if (!np)
+		goto err;
+
+	c = of_clk_get_by_name(np, NULL);
+	if (IS_ERR(c))
+		goto err;
+
+	rate = clk_get_rate(c);
+
+	clk_put(c);
+err:
+	return rate;
+}
+
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
 	u32 cpuid;
+	unsigned long rate;
 
 	for_each_online_cpu(i) {
 		/*
@@ -1194,6 +1217,12 @@ static int c_show(struct seq_file *m, void *v)
 		seq_printf(m, "model name\t: %s rev %d (%s)\n",
 			   cpu_name, cpuid & 15, elf_platform);
 
+		rate = cpu_freq(i);
+		if (rate)
+			/* Change from Hz into MHz */
+			seq_printf(m, "cpu MHz\t\t: %lu.%03lu\n",
+				   rate / 1000000, rate / 1000 % 1000);
+
 #if defined(CONFIG_SMP)
 		seq_printf(m, "BogoMIPS\t: %lu.%02lu\n",
 			   per_cpu(cpu_data, i).loops_per_jiffy / (500000UL/HZ),
-- 
1.9.1

Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

[Sudeep Holla]

On 07/06/16 22:08, Jon Mason wrote:
> Query the CPU core clock in the device tree to determine the core clock
> speed.

How do guarantee that it's the current frequency of the CPU ?
It doesn't even represent the mix or max frequency, so it's incorrect.
Some DTs have boot frequency in that entry.

> Output this clock rate in /proc/cpuinfo to match the output
> from other architectures.  The output is intentionally patterned after
> the x86 output, to match existing (and possibly expected) convention.
>
> If any errors are encountered in querying the clock (or the speed is
> erroneously zero), nothing will be printed out.  Thus any existing
> devices that do not have CPU clocks defined in the device tree will
> work as before.
>

What if they just don't have in DT but have DVFS support ?

Also whey do we need this support when the user-space can query the
CPUFreq sysfs which is more accurate and maintains the current running
frequency ?

-- 
Regards,
Sudeep

Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

[Jon Mason]

On Wed, Jun 08, 2016 at 09:34:06AM +0100, Sudeep Holla wrote:
> 
> 
> On 07/06/16 22:08, Jon Mason wrote:
> >Query the CPU core clock in the device tree to determine the core clock
> >speed.
> 
> How do guarantee that it's the current frequency of the CPU ?

I am basing it on the assumption (perhaps incorrect) that the clock in
the CPU DT corresponds to the one determining the CPU clock rate.  And,
that this clock rate is accurate in describing the speed at which the
CPU is currently running.

> It doesn't even represent the mix or max frequency, so it's incorrect.
> Some DTs have boot frequency in that entry.
> 
> >Output this clock rate in /proc/cpuinfo to match the output
> >from other architectures.  The output is intentionally patterned after
> >the x86 output, to match existing (and possibly expected) convention.
> >
> >If any errors are encountered in querying the clock (or the speed is
> >erroneously zero), nothing will be printed out.  Thus any existing
> >devices that do not have CPU clocks defined in the device tree will
> >work as before.
> >
> 
> What if they just don't have in DT but have DVFS support ?

This can be extended to cover DVFS or SMC calls or anything else.
This was simply a first step to cover what appeared to be the most
prevalent case.

> Also whey do we need this support when the user-space can query the
> CPUFreq sysfs which is more accurate and maintains the current running
> frequency ?

This is exactly what x86 is doing to provide its value in
/proc/cpuinfo.  I could easily augment this patch to call
cpufreq_quick_get(), if it returns 0, then call clk_get_rate().  If
both return 0, then simply not print out anything (which would cover
all of the possibilities).  Or, I could have it just call
cpufreq_quick_get() to get the value.

Thanks,
Jon

> 
> -- 
> Regards,
> Sudeep

Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

[Sudeep Holla]

On 08/06/16 20:31, Jon Mason wrote:
> On Wed, Jun 08, 2016 at 09:34:06AM +0100, Sudeep Holla wrote:
>>
>>
>> On 07/06/16 22:08, Jon Mason wrote:
>>> Query the CPU core clock in the device tree to determine the core clock
>>> speed.
>>
>> How do guarantee that it's the current frequency of the CPU ?
>
> I am basing it on the assumption (perhaps incorrect) that the clock in
> the CPU DT corresponds to the one determining the CPU clock rate.  And,
> that this clock rate is accurate in describing the speed at which the
> CPU is currently running.
>

As you already noticed, it's not always correct.

[..]

>>
>> What if they just don't have in DT but have DVFS support ?
>
> This can be extended to cover DVFS or SMC calls or anything else.
> This was simply a first step to cover what appeared to be the most
> prevalent case.
>

Using DVFS/CPUFreq makes this DT based approach irrelevant.

>> Also whey do we need this support when the user-space can query the
>> CPUFreq sysfs which is more accurate and maintains the current running
>> frequency ?
>
> This is exactly what x86 is doing to provide its value in
> /proc/cpuinfo.  I could easily augment this patch to call
> cpufreq_quick_get(), if it returns 0, then call clk_get_rate().  If
> both return 0, then simply not print out anything (which would cover
> all of the possibilities).  Or, I could have it just call
> cpufreq_quick_get() to get the value.
>

Agree x86 has, may be for legacy reasons. It even has CPUFreq sysfs
entries which is architecture agnostic while /proc/cpuinfo is more
architecture based. So applications that want to be portable across
architectures must choose the generic CPUFreq sysfs path rather than
some x86 based cpuinfo.

-- 
Regards,
Sudeep

Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

[Jon Mason]

On Thu, Jun 9, 2016 at 5:09 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 08/06/16 20:31, Jon Mason wrote:
>>
>> On Wed, Jun 08, 2016 at 09:34:06AM +0100, Sudeep Holla wrote:
>>>
>>>
>>>
>>> On 07/06/16 22:08, Jon Mason wrote:
>>>>
>>>> Query the CPU core clock in the device tree to determine the core clock
>>>> speed.
>>>
>>>
>>> How do guarantee that it's the current frequency of the CPU ?
>>
>>
>> I am basing it on the assumption (perhaps incorrect) that the clock in
>> the CPU DT corresponds to the one determining the CPU clock rate.  And,
>> that this clock rate is accurate in describing the speed at which the
>> CPU is currently running.
>>
>
> As you already noticed, it's not always correct.
>
> [..]
>
>>>
>>> What if they just don't have in DT but have DVFS support ?
>>
>>
>> This can be extended to cover DVFS or SMC calls or anything else.
>> This was simply a first step to cover what appeared to be the most
>> prevalent case.
>>
>
> Using DVFS/CPUFreq makes this DT based approach irrelevant.
>
>>> Also whey do we need this support when the user-space can query the
>>> CPUFreq sysfs which is more accurate and maintains the current running
>>> frequency ?
>>
>>
>> This is exactly what x86 is doing to provide its value in
>> /proc/cpuinfo.  I could easily augment this patch to call
>> cpufreq_quick_get(), if it returns 0, then call clk_get_rate().  If
>> both return 0, then simply not print out anything (which would cover
>> all of the possibilities).  Or, I could have it just call
>> cpufreq_quick_get() to get the value.
>>
>
> Agree x86 has, may be for legacy reasons. It even has CPUFreq sysfs
> entries which is architecture agnostic while /proc/cpuinfo is more
> architecture based. So applications that want to be portable across
> architectures must choose the generic CPUFreq sysfs path rather than
> some x86 based cpuinfo.

Thank you for educating me.  I am taking this (and RMK's comment) as
any modification to add CPU speed to /proc/cpuinfo is not welcomed,
anyone who wants to query this should instead look at cpufreq in
sysfs, and any dev who wants to add such a thing should look into
writing a driver in drivers/cpufreq/.

Thanks,
Jon

>
> --
> Regards,
> Sudeep

Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Russell King - ARM Linux]

On Tue, Jun 07, 2016 at 05:08:32PM -0400, Jon Mason wrote:
> Many users (and some applications) are expecting the CPU clock speed to
> be output in /proc/cpuinfo (as is done in x86, avr32, c6x, tile, parisc,
> ia64, and xtensa).

Such as what applications?  This is just another meaningless number,
which is just as meaningless as the bogomips number.  It tells you
nothing really about the CPU which should remotely be used for
anything other than user display.  It certainly can't be used for
algorithmic selection.

We have resisted publishing this information for years because not
every ARM CPU is capable of providing this information - for many, we
don't know what the CPU clock rate even is.  I believe it is a mistake
to publish this information.  If userspace wants to select an algorithm,
that needs to be done according to much more information than just the
CPU speed - it needs knowledge of the instruction timings as well, cache
behaviour, etc, and you might as well benchmark an implementation and
select at run time, caching the result.

Since we've never exported this information, it's not a regression
and it's not part of the kernels standard API.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Jon Mason]

On Tue, Jun 07, 2016 at 11:18:10PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 07, 2016 at 05:08:32PM -0400, Jon Mason wrote:
> > Many users (and some applications) are expecting the CPU clock speed to
> > be output in /proc/cpuinfo (as is done in x86, avr32, c6x, tile, parisc,
> > ia64, and xtensa).
> 
> Such as what applications?  This is just another meaningless number,

To be honest, I don't have any direct knowledge of this.  I saw it in
passing while googling to see if anyone had pushed a patch like this
before.  Lots of people complaining and asking for help on message
boards.  I think it has been mostly "fixed" by those apps now using
bogomips, but per your comment below, that is not optimal.

> which is just as meaningless as the bogomips number.  It tells you
> nothing really about the CPU which should remotely be used for
> anything other than user display.  It certainly can't be used for
> algorithmic selection.

As far as being something useful, it does have some benefits.  It can
tell you the speed the core is currently running at, which is
beneficial when trying to determine if the power management is
stepping up/down the core based on load, etc.  This could be queried
via the clk_summary in debugfs, but this is not always enabled.

Also, Linux developers/users and (more important to me) customers
coming to ARM from x86 are expecting this to be there.  While it is
completely fair to tell them "this is ARM, it is different, get used
to it", it will not stop them from asking.

> We have resisted publishing this information for years because not
> every ARM CPU is capable of providing this information - for many, we
> don't know what the CPU clock rate even is.  I believe it is a mistake
> to publish this information.  If userspace wants to select an algorithm,
> that needs to be done according to much more information than just the
> CPU speed - it needs knowledge of the instruction timings as well, cache
> behaviour, etc, and you might as well benchmark an implementation and
> select at run time, caching the result.
> 
> Since we've never exported this information, it's not a regression
> and it's not part of the kernels standard API.

I do not think it is a regression, and I'm sorry if I implied it.  

For many boards, the information is already there.  From a technical
perspective, it is no big feat to query and print it (and make people
happy).  For the boards that do not have it (or it is not relevant),
we can say "this is ARM, it is different, get used to it".

Thanks,
Jon

> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Jon Masters]

Hi Russell, Jon,

On 06/07/2016 06:18 PM, Russell King - ARM Linux wrote:
> On Tue, Jun 07, 2016 at 05:08:32PM -0400, Jon Mason wrote:
>> Many users (and some applications) are expecting the CPU clock speed to
>> be output in /proc/cpuinfo (as is done in x86, avr32, c6x, tile, parisc,
>> ia64, and xtensa).
> 
> Such as what applications?  This is just another meaningless number,
> which is just as meaningless as the bogomips number.  It tells you
> nothing really about the CPU which should remotely be used for
> anything other than user display.  It certainly can't be used for
> algorithmic selection.

Agreed. HOWEVER...

We'll be coming back to add this for servers soon enough. It's on the
todo, we just need a nice way to extract this information (as has been
discussed). That's likely to be through DMI data or an ACPI addition
that will be required and mandatory at some future point.


> We have resisted publishing this information for years because not
> every ARM CPU is capable of providing this information - for many, we
> don't know what the CPU clock rate even is.  I believe it is a mistake
> to publish this information.

There are two specific problems on ARM servers:

1). Idiots. I've seen many situations in which benchmarks were run on
pre-production machines (often explicitly against legal agreements, but
those never happened within my organization so that's their problem -
this is carefully prohibited activity within our own walls and I make
sure our guys never run any numbers on pre-production stuff just so it
could never get into the "wrong" hands by accident...). What these
idiots do is then spread rumors that ARM server X "sucks" because they
ran a prohibited benchmark against a downclocked core and forgot how to
multiply numbers together. It gets worse. But usually it's because they
ran "cat /proc/cpuinfo" and it didn't tell them anything useful, so they
used whatever they thought the frequency was supposed to be.

These idiots include journalists. If you read the press, you'll see the
Gandhi sequence is playing out already between one of the established
vendors and an ARM vendor in which said established vendor is at the
laughing/fighting stage of things. And in a couple of cases recently,
the press included "we didn't know how fast it ran because /proc/cpuinfo
didn't tell us anything, so we guessed".

2). Users. Those who use and admin servers get the "speed" from
/proc/cpuinfo. It's a marketing number. It's useless. It absolutely must
be provided to the user or administrator exactly like on x86.

> If userspace wants to select an algorithm,
> that needs to be done according to much more information than just the
> CPU speed - it needs knowledge of the instruction timings as well, cache
> behaviour, etc, and you might as well benchmark an implementation and
> select at run time, caching the result.
> 
> Since we've never exported this information, it's not a regression
> and it's not part of the kernels standard API.

Agreed. But we'll still be coming back to ensure this information is
presented to users. I pointed out to ARM about 3-4 years ago that this
was going to bite us. It is now biting us, and we will ensure that
useless data is provided where it is on x86 for identical experience by
users. That is unless or until x86 users do something else always. Our
(separate) case will use DMI or ACPI for the same kind of data.

Jon.

Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Russell King - ARM Linux]

On Sat, Jul 02, 2016 at 07:58:00PM -0400, Jon Masters wrote:
> Agreed. But we'll still be coming back to ensure this information is
> presented to users. I pointed out to ARM about 3-4 years ago that this
> was going to bite us. It is now biting us, and we will ensure that
> useless data is provided where it is on x86 for identical experience by
> users. That is unless or until x86 users do something else always. Our
> (separate) case will use DMI or ACPI for the same kind of data.

Right, so having read all your email, there's no reason why we couldn't
just print:

cpu MHz  : 99999999.999

and be done with it.  Sure, it doesn't reflect the reality, but if people
are going to be idiots with it, why not play their game with it to show
how stupid it is.

I don't buy "it's biting us" - I see no evidence of it actually "biting"
anyone.  No one has reported any failures in the last 20 years due to
this missing - and even so as I've already said, it would _not_ be a
regression because that information has never been provided on 32-bit
ARM.

Moreover, I asked what these applications were that are affected by the
lack of us providing this number.  I'm still waiting to hear that, and
I noticed that even you skipped over providing that information which I
asked for, instead giving a hand-wavey answer based on marketing (spit)
people doing stupid things.

Please, try to come up with a _technical_ justification.  I really don't
want to make decisions based on marketing shite which I completely and
utterly despise.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Andrew Lunn]

On Sun, Jul 03, 2016 at 05:54:31PM +0100, Russell King - ARM Linux wrote:
> On Sat, Jul 02, 2016 at 07:58:00PM -0400, Jon Masters wrote:
> > Agreed. But we'll still be coming back to ensure this information is
> > presented to users. I pointed out to ARM about 3-4 years ago that this
> > was going to bite us. It is now biting us, and we will ensure that
> > useless data is provided where it is on x86 for identical experience by
> > users. That is unless or until x86 users do something else always. Our
> > (separate) case will use DMI or ACPI for the same kind of data.
> 
> Right, so having read all your email, there's no reason why we couldn't
> just print:
> 
> cpu MHz  : 99999999.999

Since it is a float, how about using the value NaN? I think that
nicely summaries it is a useless value.

However, i agree, we first need a technical justification for needing
a value at all.

Jon: What happened when you posted a patch removing this value from
x86? That is clearly an alternative to adding it to ARM, especially if
everybody agrees it is a useless value.

  Andrew

Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Russell King - ARM Linux]

On Sun, Jul 03, 2016 at 08:49:45PM +0200, Andrew Lunn wrote:
> On Sun, Jul 03, 2016 at 05:54:31PM +0100, Russell King - ARM Linux wrote:
> > Right, so having read all your email, there's no reason why we couldn't
> > just print:
> > 
> > cpu MHz  : 99999999.999
> 
> Since it is a float, how about using the value NaN? I think that
> nicely summaries it is a useless value.

;)

> However, i agree, we first need a technical justification for needing
> a value at all.

The only case where we've had a userspace "failure" is with the Jack
audio server, which wanted to parse the cpu MHz value to use it in
this calculation:

     static jack_time_t jack_get_microseconds_from_cycles (void) {
      return get_cycles() / __jack_cpu_mhz;
     }

Now, let's say that we did provide the CPU MHz, so __jack_cpu_mhz
reflects this value.  Great, Jack can initialise this value, but
there's two fundamental problems:

1. Jack is not aware of cpufreq, so the CPU MHz value it read at
   one point in time may not be the current CPU MHz - the CPU
   frequency can change at any moment depending on the governor's
   decisions.

2. get_cycles()... we have no userspace accessible CPU cycle counters
   on 32-bit ARM, which means knowing the CPU MHz for use like this
   gets you nowhere as get_cycles() can't return the number of CPU
   cycles.  It certainly can't return a number based on the CPU MHz.

Hence why this change to jack was the only sane thing to do:

https://github.com/jackaudio/jack2/commit/d425d8035b761b4a362c538c41eca874ff4

This wasn't even a kernel regression - the kernel never provided the
value on ARM, and in many cases the kernel doesn't know the right
value (as I've said previously in this thread.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo

[Pavel Machek]

On Sun 2016-07-03 17:54:31, Russell King - ARM Linux wrote:
> On Sat, Jul 02, 2016 at 07:58:00PM -0400, Jon Masters wrote:
> > Agreed. But we'll still be coming back to ensure this information is
> > presented to users. I pointed out to ARM about 3-4 years ago that this
> > was going to bite us. It is now biting us, and we will ensure that
> > useless data is provided where it is on x86 for identical experience by
> > users. That is unless or until x86 users do something else always. Our
> > (separate) case will use DMI or ACPI for the same kind of data.
> 
> Right, so having read all your email, there's no reason why we couldn't
> just print:
> 
> cpu MHz  : 99999999.999

You apparenly did not read the email. Because bogus number will not
help people detecting underclocked configurations, as email was
explaining.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html