diff mbox series

[v3,5/7] cpufreq: add function to get the hardware max frequency

Message ID 20200211184542.29585-6-ionela.voinescu@arm.com (mailing list archive)
State Not Applicable, archived
Headers show
Series arm64: ARMv8.4 Activity Monitors support | expand

Commit Message

Ionela Voinescu Feb. 11, 2020, 6:45 p.m. UTC
Add weak function to return the hardware maximum frequency of a
CPU, with the default implementation returning cpuinfo.max_freq.

The default can be overwritten by a strong function in platforms
that want to provide an alternative implementation.

Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 20 ++++++++++++++++++++
 include/linux/cpufreq.h   |  5 +++++
 2 files changed, 25 insertions(+)

Comments

Viresh Kumar Feb. 12, 2020, 4:14 a.m. UTC | #1
On 11-02-20, 18:45, Ionela Voinescu wrote:
> Add weak function to return the hardware maximum frequency of a
> CPU, with the default implementation returning cpuinfo.max_freq.
> 
> The default can be overwritten by a strong function in platforms
> that want to provide an alternative implementation.
> 
> Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 20 ++++++++++++++++++++
>  include/linux/cpufreq.h   |  5 +++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 77114a3897fb..d2ff3018861d 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1733,6 +1733,26 @@ unsigned int cpufreq_quick_get_max(unsigned int cpu)
>  }
>  EXPORT_SYMBOL(cpufreq_quick_get_max);
>  
> +/**
> + * cpufreq_get_hw_max_freq - get the max hardware frequency of the CPU
> + * @cpu: CPU number
> + *
> + * The default return value is the max_freq field of cpuinfo.
> + */
> +__weak unsigned int cpufreq_get_hw_max_freq(unsigned int cpu)
> +{
> +	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> +	unsigned int ret_freq = 0;
> +
> +	if (policy) {
> +		ret_freq = policy->cpuinfo.max_freq;
> +		cpufreq_cpu_put(policy);
> +	}
> +
> +	return ret_freq;
> +}
> +EXPORT_SYMBOL(cpufreq_get_hw_max_freq);
> +
>  static unsigned int __cpufreq_get(struct cpufreq_policy *policy)
>  {
>  	if (unlikely(policy_is_inactive(policy)))
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 31b1b0e03df8..b4423ff619f4 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -194,6 +194,7 @@ extern struct kobject *cpufreq_global_kobject;
>  unsigned int cpufreq_get(unsigned int cpu);
>  unsigned int cpufreq_quick_get(unsigned int cpu);
>  unsigned int cpufreq_quick_get_max(unsigned int cpu);
> +unsigned int cpufreq_get_hw_max_freq(unsigned int cpu);
>  void disable_cpufreq(void);
>  
>  u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy);
> @@ -223,6 +224,10 @@ static inline unsigned int cpufreq_quick_get_max(unsigned int cpu)
>  {
>  	return 0;
>  }
> +static inline unsigned int cpufreq_get_hw_max_freq(unsigned int cpu)
> +{
> +	return 0;
> +}
>  static inline void disable_cpufreq(void) { }
>  #endif

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Valentin Schneider Feb. 13, 2020, 11:59 a.m. UTC | #2
On 2/11/20 6:45 PM, Ionela Voinescu wrote:
> +/**
> + * cpufreq_get_hw_max_freq - get the max hardware frequency of the CPU
> + * @cpu: CPU number
> + *
> + * The default return value is the max_freq field of cpuinfo.
> + */
> +__weak unsigned int cpufreq_get_hw_max_freq(unsigned int cpu)
> +{
> +	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> +	unsigned int ret_freq = 0;
> +
> +	if (policy) {
> +		ret_freq = policy->cpuinfo.max_freq;
> +		cpufreq_cpu_put(policy);

What about intel_pstate / turbo stuff? IIRC one of Giovanni's issues was that
turbo freq is not always reported as the max freq. Dunno if we can do
anything about it; at the very least maybe document the caveat?

> +	}
> +
> +	return ret_freq;
> +}
> +EXPORT_SYMBOL(cpufreq_get_hw_max_freq);
> +
>  static unsigned int __cpufreq_get(struct cpufreq_policy *policy)
>  {
>  	if (unlikely(policy_is_inactive(policy)))
Ionela Voinescu Feb. 13, 2020, 12:59 p.m. UTC | #3
On Thursday 13 Feb 2020 at 11:59:56 (+0000), Valentin Schneider wrote:
> On 2/11/20 6:45 PM, Ionela Voinescu wrote:
> > +/**
> > + * cpufreq_get_hw_max_freq - get the max hardware frequency of the CPU
> > + * @cpu: CPU number
> > + *
> > + * The default return value is the max_freq field of cpuinfo.
> > + */
> > +__weak unsigned int cpufreq_get_hw_max_freq(unsigned int cpu)
> > +{
> > +	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> > +	unsigned int ret_freq = 0;
> > +
> > +	if (policy) {
> > +		ret_freq = policy->cpuinfo.max_freq;
> > +		cpufreq_cpu_put(policy);
> 
> What about intel_pstate / turbo stuff? IIRC one of Giovanni's issues was that
> turbo freq is not always reported as the max freq. Dunno if we can do
> anything about it; at the very least maybe document the caveat?
>

Okay, I can add details in the description in regards to potential
reasons to overwrite this function. But basically this is one of the
reasons for making this a weak function. The best information we can
generically get for maximum hardware frequency is cpuinfo.max_freq.
But if platforms have the possibility to obtain this differently from
either hardware or firmware they can overwrite this.

Thanks,
Ionela.

> > +	}
> > +
> > +	return ret_freq;
> > +}
> > +EXPORT_SYMBOL(cpufreq_get_hw_max_freq);
> > +
> >  static unsigned int __cpufreq_get(struct cpufreq_policy *policy)
> >  {
> >  	if (unlikely(policy_is_inactive(policy)))
>
Valentin Schneider Feb. 13, 2020, 3:22 p.m. UTC | #4
On 2/13/20 12:59 PM, Ionela Voinescu wrote:
>> What about intel_pstate / turbo stuff? IIRC one of Giovanni's issues was that
>> turbo freq is not always reported as the max freq. Dunno if we can do
>> anything about it; at the very least maybe document the caveat?
>>
> 
> Okay, I can add details in the description in regards to potential
> reasons to overwrite this function. But basically this is one of the
> reasons for making this a weak function. The best information we can
> generically get for maximum hardware frequency is cpuinfo.max_freq.
> But if platforms have the possibility to obtain this differently from
> either hardware or firmware they can overwrite this.
> 

Right, that would be handled by a different implementation of
that function, so this wasn't too relevant of a comment. Sorry!
diff mbox series

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 77114a3897fb..d2ff3018861d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1733,6 +1733,26 @@  unsigned int cpufreq_quick_get_max(unsigned int cpu)
 }
 EXPORT_SYMBOL(cpufreq_quick_get_max);
 
+/**
+ * cpufreq_get_hw_max_freq - get the max hardware frequency of the CPU
+ * @cpu: CPU number
+ *
+ * The default return value is the max_freq field of cpuinfo.
+ */
+__weak unsigned int cpufreq_get_hw_max_freq(unsigned int cpu)
+{
+	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+	unsigned int ret_freq = 0;
+
+	if (policy) {
+		ret_freq = policy->cpuinfo.max_freq;
+		cpufreq_cpu_put(policy);
+	}
+
+	return ret_freq;
+}
+EXPORT_SYMBOL(cpufreq_get_hw_max_freq);
+
 static unsigned int __cpufreq_get(struct cpufreq_policy *policy)
 {
 	if (unlikely(policy_is_inactive(policy)))
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 31b1b0e03df8..b4423ff619f4 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -194,6 +194,7 @@  extern struct kobject *cpufreq_global_kobject;
 unsigned int cpufreq_get(unsigned int cpu);
 unsigned int cpufreq_quick_get(unsigned int cpu);
 unsigned int cpufreq_quick_get_max(unsigned int cpu);
+unsigned int cpufreq_get_hw_max_freq(unsigned int cpu);
 void disable_cpufreq(void);
 
 u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy);
@@ -223,6 +224,10 @@  static inline unsigned int cpufreq_quick_get_max(unsigned int cpu)
 {
 	return 0;
 }
+static inline unsigned int cpufreq_get_hw_max_freq(unsigned int cpu)
+{
+	return 0;
+}
 static inline void disable_cpufreq(void) { }
 #endif