Re: [PATCH RFC] intel_pstate: play well with frequency limits set by acpi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2015-07-17 at 07:36 +0300, Konstantin Khlebnikov wrote:
> On Fri, Jul 17, 2015 at 1:08 AM, Srinivas Pandruvada
> <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
> > On Thu, 2015-07-16 at 21:17 +0300, Konstantin Khlebnikov wrote:
> >> IPMI can control CPU P-states remotely: configuration is reported via
> >> common ACPI interface (_PPC/_PSS/etc). This patch adds required minimal
> >> support in intel_pstate to receive and use these P-state limits.
> >>
> >> * ignore limit of top state in _PPC: it lower than turbo boost frequency
> >> * register intel_pstate in acpi-processor to get states from _PSS
> >> * link acpi_processor_get_bios_limit: this adds attribute "bios_limit"
> >>
> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
> >> ---
> >>  drivers/acpi/processor_perflib.c |    3 +-
> >>  drivers/cpufreq/intel_pstate.c   |   57 ++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 59 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> >> index cfc8aba72f86..781e328c9d5f 100644
> >> --- a/drivers/acpi/processor_perflib.c
> >> +++ b/drivers/acpi/processor_perflib.c
> >> @@ -98,7 +98,8 @@ static int acpi_processor_ppc_notifier(struct notifier_block *nb,
> >>
> >>       ppc = (unsigned int)pr->performance_platform_limit;
> >>
> >> -     if (ppc >= pr->performance->state_count)
> >> +     /* Ignore limit of top state: it lower than turbo boost frequency */
> >> +     if (!ppc || ppc >= pr->performance->state_count)
> > Why? Isn't the previous check enough?
> 
> Zero _PPC state must be top performance state but as I see frequency in
> _PSS is lower than maximum possible turbo frequency. So, in this case
> intel_pstate cannnot get "100%"  for max bound even it there is no limit set.
> 
> For example: I saw _PSS[0] = 2601 Mhz, PSS[1] = 2600 Mhz while turbo
> state is 3400 Mhz.
> 
Have you tested dynamic _PPC modification with acpi cpufreq with this
change (after boot)? Suppose _PPC is changed from 3 to 0, then
cpufreq_verify_within_limits will not be called to change to new max
turbo performance state.

Thanks,
Srinivas
> >>               goto out;
> >>
> >>       cpufreq_verify_within_limits(policy, 0,
> >> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> >> index 15ada47bb720..4a34ddf4fa73 100644
> >> --- a/drivers/cpufreq/intel_pstate.c
> >> +++ b/drivers/cpufreq/intel_pstate.c
> >> @@ -26,6 +26,7 @@
> >>  #include <linux/fs.h>
> >>  #include <linux/debugfs.h>
> >>  #include <linux/acpi.h>
> >> +#include <acpi/processor.h>
> >>  #include <linux/vmalloc.h>
> >>  #include <trace/events/power.h>
> >>
> >> @@ -113,6 +114,9 @@ struct cpudata {
> >>       u64     prev_mperf;
> >>       u64     prev_tsc;
> >>       struct sample sample;
> >> +#ifdef CONFIG_ACPI_PROCESSOR
> >> +     struct acpi_processor_performance acpi_data;
> >> +#endif
> >>  };
> >>
> >>  static struct cpudata **all_cpu_data;
> >> @@ -145,6 +149,7 @@ static int hwp_active;
> >>
> >>  struct perf_limits {
> >>       int no_turbo;
> >> +     int no_acpi;
> >>       int turbo_disabled;
> >>       int max_perf_pct;
> >>       int min_perf_pct;
> >> @@ -158,6 +163,7 @@ struct perf_limits {
> >>
> >>  static struct perf_limits limits = {
> >>       .no_turbo = 0,
> >> +     .no_acpi = !IS_ENABLED(CONFIG_ACPI_PROCESSOR),
> >>       .turbo_disabled = 0,
> >>       .max_perf_pct = 100,
> >>       .max_perf = int_tofp(1),
> >> @@ -449,6 +455,18 @@ static ssize_t store_min_perf_pct(struct kobject *a, struct attribute *b,
> >>       return count;
> >>  }
> >>
> >> +static ssize_t store_no_acpi(struct kobject *a, struct attribute *b,
> >> +                          const char *buf, size_t count)
> >> +{
> >> +#ifdef CONFIG_ACPI_PROCESSOR
> >> +     return kstrtouint(buf, 0, &limits.no_acpi) ?: count;
> >> +#else
> >> +     return -ENODEV;
> >> +#endif
> >> +}
> >> +show_one(no_acpi, no_acpi);
> >> +define_one_global_rw(no_acpi);
> >> +
> >>  show_one(max_perf_pct, max_perf_pct);
> >>  show_one(min_perf_pct, min_perf_pct);
> >>
> >> @@ -460,6 +478,7 @@ define_one_global_ro(num_pstates);
> >>
> >>  static struct attribute *intel_pstate_attributes[] = {
> >>       &no_turbo.attr,
> >> +     &no_acpi.attr,
> >>       &max_perf_pct.attr,
> >>       &min_perf_pct.attr,
> >>       &turbo_pct.attr,
> >> @@ -1049,6 +1068,38 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
> >>       policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
> >>       cpumask_set_cpu(policy->cpu, policy->cpus);
> >>
> >> +#ifdef CONFIG_ACPI_PROCESSOR
> >> +     if (!limits.no_acpi) {
> >> +             /*
> >> +              * Minimum necessary to get acpi_processor_ppc_notifier() and
> >> +              * acpi_processor_get_bios_limit() working.
> >> +              */
> >> +             if (!zalloc_cpumask_var(&cpu->acpi_data.shared_cpu_map,
> >> +                                     GFP_KERNEL))
> >> +                     rc = -ENOMEM;
> >> +             else
> >> +                     rc = acpi_processor_register_performance(
> >> +                                     &cpu->acpi_data, policy->cpu);
> >> +             if (rc) {
> >> +                     pr_err("intel_pstate: acpi init failed: %d\n", rc);
> >> +                     free_cpumask_var(cpu->acpi_data.shared_cpu_map);
> >> +                     limits.no_acpi = 1;
> >> +             }
> >> +     }
> >> +#endif
> >> +     return 0;
> >> +}
> >> +
> >> +static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
> >> +{
> >> +#ifdef CONFIG_ACPI_PROCESSOR
> >> +     struct cpudata *cpu = all_cpu_data[policy->cpu];
> >> +
> >> +     if (cpu->acpi_data.state_count)
> >> +             acpi_processor_unregister_performance(&cpu->acpi_data,
> >> +                                                   policy->cpu);
> >> +     free_cpumask_var(cpu->acpi_data.shared_cpu_map);
> >> +#endif
> >>       return 0;
> >>  }
> >>
> >> @@ -1057,7 +1108,11 @@ static struct cpufreq_driver intel_pstate_driver = {
> >>       .verify         = intel_pstate_verify_policy,
> >>       .setpolicy      = intel_pstate_set_policy,
> >>       .get            = intel_pstate_get,
> >> +#ifdef CONFIG_ACPI_PROCESSOR
> >> +     .bios_limit     = acpi_processor_get_bios_limit,
> >> +#endif
> >>       .init           = intel_pstate_cpu_init,
> >> +     .exit           = intel_pstate_cpu_exit,
> >>       .stop_cpu       = intel_pstate_stop_cpu,
> >>       .name           = "intel_pstate",
> >>  };
> >> @@ -1286,6 +1341,8 @@ static int __init intel_pstate_setup(char *str)
> >>               force_load = 1;
> >>       if (!strcmp(str, "hwp_only"))
> >>               hwp_only = 1;
> >> +     if (!strcmp(str, "no_acpi"))
> >> +             limits.no_acpi = 1;
> >>       return 0;
> >>  }
> >>  early_param("intel_pstate", intel_pstate_setup);
> >>
> > _PPC is index into _PSS. Since intel P state doesn't follow _PSS, the
> > states may not be 1:1 matching. So we have to harmonize them.
> >
> >
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux