* [PATCH 0/3] Intel Pstate: ACPI perf integration issues
@ 2015-11-10 2:58 Srinivas Pandruvada
2015-11-10 2:58 ` [PATCH 1/3] cpufreq: intel_pstate: Don't use ACPI perf for HWP Srinivas Pandruvada
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Srinivas Pandruvada @ 2015-11-10 2:58 UTC (permalink / raw)
To: rafael.j.wysocki, lenlenb; +Cc: linux-pm, Srinivas Pandruvada
Some minor fixes related to ACPI _PSS.
Main one is ignore _PSS and _PPC when HWP (hardware P state) is enabled.
Srinivas Pandruvada (3):
cpufreq: intel_pstate: Don't use ACPI perf for HWP
cpufreq: intel_pstate: wrong flag checking for no_acpi_perf
cpufreq: intel_pstate: Fix the scaling of _PSS entry[0]
drivers/cpufreq/intel_pstate.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] cpufreq: intel_pstate: Don't use ACPI perf for HWP 2015-11-10 2:58 [PATCH 0/3] Intel Pstate: ACPI perf integration issues Srinivas Pandruvada @ 2015-11-10 2:58 ` Srinivas Pandruvada 2015-11-14 0:44 ` Rafael J. Wysocki 2015-11-10 2:58 ` [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf Srinivas Pandruvada 2015-11-10 2:58 ` [PATCH 3/3] cpufreq: intel_pstate: Fix the scaling of _PSS entry[0] Srinivas Pandruvada 2 siblings, 1 reply; 8+ messages in thread From: Srinivas Pandruvada @ 2015-11-10 2:58 UTC (permalink / raw) To: rafael.j.wysocki, lenlenb; +Cc: linux-pm, Srinivas Pandruvada When HWP is enabled, the information in _PSS and _PPC prevents entering turbo states. The reason is HWP and legacy pstate are not compatible. There is no processing of turbo activation ratio in HWP, but _PPC setting still behaves as there is support. So we end up in a pstate, which is below or equal to max non turbo ratio. Also the max turbo ratio is set to wrong value in _PSS, so even if HWP is disabled to use acpi-cpufrq or intel_pstate in legacy mode, this will still cause issue. This change will not read any _PSS table or call acpi_processor_register_performance, when the platform has HWP feature. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> --- drivers/cpufreq/intel_pstate.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 93a3c63..a0b8180 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -1557,8 +1557,11 @@ static int __init intel_pstate_init(void) if (!all_cpu_data) return -ENOMEM; - if (static_cpu_has_safe(X86_FEATURE_HWP) && !no_hwp) - hwp_active++; + if (static_cpu_has_safe(X86_FEATURE_HWP)) { + if (!no_hwp) + hwp_active++; + no_acpi_perf = 1; + } if (!hwp_active && hwp_only) goto out; -- 1.9.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] cpufreq: intel_pstate: Don't use ACPI perf for HWP 2015-11-10 2:58 ` [PATCH 1/3] cpufreq: intel_pstate: Don't use ACPI perf for HWP Srinivas Pandruvada @ 2015-11-14 0:44 ` Rafael J. Wysocki 0 siblings, 0 replies; 8+ messages in thread From: Rafael J. Wysocki @ 2015-11-14 0:44 UTC (permalink / raw) To: Srinivas Pandruvada; +Cc: rafael.j.wysocki, lenlenb, linux-pm On Monday, November 09, 2015 06:58:09 PM Srinivas Pandruvada wrote: > When HWP is enabled, the information in _PSS and _PPC prevents entering > turbo states. The reason is HWP and legacy pstate are not compatible. > There is no processing of turbo activation ratio in HWP, but _PPC setting > still behaves as there is support. So we end up in a pstate, which is > below or equal to max non turbo ratio. Also the max turbo ratio is set to > wrong value in _PSS, so even if HWP is disabled to use acpi-cpufrq or > intel_pstate in legacy mode, this will still cause issue. This change will > not read any _PSS table or call acpi_processor_register_performance, > when the platform has HWP feature. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > drivers/cpufreq/intel_pstate.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > index 93a3c63..a0b8180 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -1557,8 +1557,11 @@ static int __init intel_pstate_init(void) > if (!all_cpu_data) > return -ENOMEM; > > - if (static_cpu_has_safe(X86_FEATURE_HWP) && !no_hwp) > - hwp_active++; > + if (static_cpu_has_safe(X86_FEATURE_HWP)) { > + if (!no_hwp) > + hwp_active++; > + no_acpi_perf = 1; > + } > > if (!hwp_active && hwp_only) > goto out; Rebased on top of the current Linus' tree and applied. Thanks, Rafael ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf 2015-11-10 2:58 [PATCH 0/3] Intel Pstate: ACPI perf integration issues Srinivas Pandruvada 2015-11-10 2:58 ` [PATCH 1/3] cpufreq: intel_pstate: Don't use ACPI perf for HWP Srinivas Pandruvada @ 2015-11-10 2:58 ` Srinivas Pandruvada 2015-11-14 0:46 ` Rafael J. Wysocki 2015-11-10 2:58 ` [PATCH 3/3] cpufreq: intel_pstate: Fix the scaling of _PSS entry[0] Srinivas Pandruvada 2 siblings, 1 reply; 8+ messages in thread From: Srinivas Pandruvada @ 2015-11-10 2:58 UTC (permalink / raw) To: rafael.j.wysocki, lenlenb; +Cc: linux-pm, Srinivas Pandruvada The checking of no_acpi_perf flag is wrong. Since there is null check in acpi_processor_unregister_performance this doesn't cause any issue when this flag is 0. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> --- drivers/cpufreq/intel_pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index a0b8180..69dc9b5 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -328,7 +328,7 @@ static int intel_pstate_exit_perf_limits(struct cpufreq_policy *policy) { struct cpudata *cpu; - if (!no_acpi_perf) + if (no_acpi_perf) return 0; cpu = all_cpu_data[policy->cpu]; -- 1.9.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf 2015-11-10 2:58 ` [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf Srinivas Pandruvada @ 2015-11-14 0:46 ` Rafael J. Wysocki 2015-11-14 0:35 ` Srinivas Pandruvada 0 siblings, 1 reply; 8+ messages in thread From: Rafael J. Wysocki @ 2015-11-14 0:46 UTC (permalink / raw) To: Srinivas Pandruvada; +Cc: rafael.j.wysocki, lenlenb, linux-pm On Monday, November 09, 2015 06:58:10 PM Srinivas Pandruvada wrote: > The checking of no_acpi_perf flag is wrong. Since there is null > check in acpi_processor_unregister_performance this doesn't cause > any issue when this flag is 0. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > drivers/cpufreq/intel_pstate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > index a0b8180..69dc9b5 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -328,7 +328,7 @@ static int intel_pstate_exit_perf_limits(struct cpufreq_policy *policy) > { > struct cpudata *cpu; > > - if (!no_acpi_perf) > + if (no_acpi_perf) > return 0; > > cpu = all_cpu_data[policy->cpu]; While the patch is correct, the changelog doesn't make a lot of sense to me to be honest. In addition to that, the cpu variable here is actually not used, so what about doing something like the below instead here? --- drivers/cpufreq/intel_pstate.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -326,13 +326,9 @@ static int intel_pstate_init_perf_limits static int intel_pstate_exit_perf_limits(struct cpufreq_policy *policy) { - struct cpudata *cpu; - if (!no_acpi_perf) - return 0; + acpi_processor_unregister_performance(policy->cpu); - cpu = all_cpu_data[policy->cpu]; - acpi_processor_unregister_performance(policy->cpu); return 0; } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf 2015-11-14 0:46 ` Rafael J. Wysocki @ 2015-11-14 0:35 ` Srinivas Pandruvada 0 siblings, 0 replies; 8+ messages in thread From: Srinivas Pandruvada @ 2015-11-14 0:35 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: rafael.j.wysocki, lenlenb, linux-pm On Sat, 2015-11-14 at 01:46 +0100, Rafael J. Wysocki wrote: > On Monday, November 09, 2015 06:58:10 PM Srinivas Pandruvada wrote: > > The checking of no_acpi_perf flag is wrong. Since there is null > > check in acpi_processor_unregister_performance this doesn't cause > > any issue when this flag is 0. > > > > Signed-off-by: Srinivas Pandruvada < > > srinivas.pandruvada@linux.intel.com> > > --- > > drivers/cpufreq/intel_pstate.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/intel_pstate.c > > b/drivers/cpufreq/intel_pstate.c > > index a0b8180..69dc9b5 100644 > > --- a/drivers/cpufreq/intel_pstate.c > > +++ b/drivers/cpufreq/intel_pstate.c > > @@ -328,7 +328,7 @@ static int intel_pstate_exit_perf_limits(struct > > cpufreq_policy *policy) > > { > > struct cpudata *cpu; > > > > - if (!no_acpi_perf) > > + if (no_acpi_perf) > > return 0; > > > > cpu = all_cpu_data[policy->cpu]; > > While the patch is correct, the changelog doesn't make a lot of sense > to me > to be honest. > > In addition to that, the cpu variable here is actually not used, so > what > about doing something like the below instead here? Sure, I will resubmit this. Thanks, Srinivas --- > drivers/cpufreq/intel_pstate.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > Index: linux-pm/drivers/cpufreq/intel_pstate.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c > +++ linux-pm/drivers/cpufreq/intel_pstate.c > @@ -326,13 +326,9 @@ static int intel_pstate_init_perf_limits > > static int intel_pstate_exit_perf_limits(struct cpufreq_policy > *policy) > { > - struct cpudata *cpu; > - > if (!no_acpi_perf) > - return 0; > + acpi_processor_unregister_performance(policy->cpu); > > - cpu = all_cpu_data[policy->cpu]; > - acpi_processor_unregister_performance(policy->cpu); > return 0; > } > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] cpufreq: intel_pstate: Fix the scaling of _PSS entry[0] 2015-11-10 2:58 [PATCH 0/3] Intel Pstate: ACPI perf integration issues Srinivas Pandruvada 2015-11-10 2:58 ` [PATCH 1/3] cpufreq: intel_pstate: Don't use ACPI perf for HWP Srinivas Pandruvada 2015-11-10 2:58 ` [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf Srinivas Pandruvada @ 2015-11-10 2:58 ` Srinivas Pandruvada 2015-11-14 0:45 ` Rafael J. Wysocki 2 siblings, 1 reply; 8+ messages in thread From: Srinivas Pandruvada @ 2015-11-10 2:58 UTC (permalink / raw) To: rafael.j.wysocki, lenlenb; +Cc: linux-pm, Srinivas Pandruvada Since even non core platforms like Atom SoC has _PSS table, we can't hardcode scaling. Instead we can use cpu->pstate.scaling. We need to convert to MHz as the scaling values are in KHz. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> --- drivers/cpufreq/intel_pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 69dc9b5..50d3cff 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -312,7 +312,7 @@ static int intel_pstate_init_perf_limits(struct cpufreq_policy *policy) * Also need to convert to MHz as _PSS freq is in MHz. */ cpu->acpi_perf_data.states[0].core_frequency = - turbo_pss_ctl * 100; + turbo_pss_ctl * cpu->pstate.scaling / 1000; } pr_debug("intel_pstate: Updated limits using _PSS 0x%x 0x%x 0x%x\n", -- 1.9.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] cpufreq: intel_pstate: Fix the scaling of _PSS entry[0] 2015-11-10 2:58 ` [PATCH 3/3] cpufreq: intel_pstate: Fix the scaling of _PSS entry[0] Srinivas Pandruvada @ 2015-11-14 0:45 ` Rafael J. Wysocki 0 siblings, 0 replies; 8+ messages in thread From: Rafael J. Wysocki @ 2015-11-14 0:45 UTC (permalink / raw) To: Srinivas Pandruvada; +Cc: rafael.j.wysocki, lenlenb, linux-pm On Monday, November 09, 2015 06:58:11 PM Srinivas Pandruvada wrote: > Since even non core platforms like Atom SoC has _PSS table, we can't > hardcode scaling. Instead we can use cpu->pstate.scaling. We need to > convert to MHz as the scaling values are in KHz. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > drivers/cpufreq/intel_pstate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > index 69dc9b5..50d3cff 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -312,7 +312,7 @@ static int intel_pstate_init_perf_limits(struct cpufreq_policy *policy) > * Also need to convert to MHz as _PSS freq is in MHz. > */ > cpu->acpi_perf_data.states[0].core_frequency = > - turbo_pss_ctl * 100; > + turbo_pss_ctl * cpu->pstate.scaling / 1000; > } > > pr_debug("intel_pstate: Updated limits using _PSS 0x%x 0x%x 0x%x\n", > Applied, thanks! Rafael ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-14 0:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-11-10 2:58 [PATCH 0/3] Intel Pstate: ACPI perf integration issues Srinivas Pandruvada 2015-11-10 2:58 ` [PATCH 1/3] cpufreq: intel_pstate: Don't use ACPI perf for HWP Srinivas Pandruvada 2015-11-14 0:44 ` Rafael J. Wysocki 2015-11-10 2:58 ` [PATCH 2/3] cpufreq: intel_pstate: wrong flag checking for no_acpi_perf Srinivas Pandruvada 2015-11-14 0:46 ` Rafael J. Wysocki 2015-11-14 0:35 ` Srinivas Pandruvada 2015-11-10 2:58 ` [PATCH 3/3] cpufreq: intel_pstate: Fix the scaling of _PSS entry[0] Srinivas Pandruvada 2015-11-14 0:45 ` Rafael J. Wysocki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.