[PATCH] Revert "cpufreq: intel_pstate: Enable PPC enforcement for servers"

[PATCH] Revert "cpufreq: intel_pstate: Enable PPC enforcement for servers"

[Srinivas Pandruvada]

This change reverts
'commit 2b3ec7650584 ("cpufreq: intel_pstate: Enable PPC enforcement for
servers")'

Intel P State uses max P-State as the max turbo P-State. This max P-State
can be limited by ACPI _PSS table entry 0. After
'commit 9522a2ff9cde ("cpufreq: intel_pstate: Enforce _PPC limits")''
the _PSS table entry[0] will be used to cap max performance for
enterprise and performance server by default.

Even though this is correct processing, but when the performance results
are compared with the version before the above commit, then obviously the
results will be worse, if the _PSS table entry 0 is not the max turbo
P-State.

So to minimize impact on performance, this revert will disable the default
enforcement of ACPI _PSS and ACPI _PPC. So this feature can only only be
explicitly activated by kernel command line
intel_pstate=support_acpi_ppc

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 Documentation/kernel-parameters.txt |  5 +----
 drivers/cpufreq/intel_pstate.c      | 15 +--------------
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 82b42c9..b7c3a56 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1683,10 +1683,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Only load intel_pstate on systems which support
 			hardware P state control (HWP) if available.
 		support_acpi_ppc
-			Enforce ACPI _PPC performance limits. If the Fixed ACPI
-			Description Table, specifies preferred power management
-			profile as "Enterprise Server" or "Performance Server",
-			then this feature is turned on by default.
+			Enforce ACPI _PPC performance limits.
 
 	intremap=	[X86-64, Intel-IOMMU]
 			on	enable Interrupt Remapping (default)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 3a9c432..518537c 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -362,16 +362,6 @@ static struct perf_limits *limits = &powersave_limits;
 #endif
 
 #ifdef CONFIG_ACPI
-
-static bool intel_pstate_get_ppc_enable_status(void)
-{
-	if (acpi_gbl_FADT.preferred_profile == PM_ENTERPRISE_SERVER ||
-	    acpi_gbl_FADT.preferred_profile == PM_PERFORMANCE_SERVER)
-		return true;
-
-	return acpi_ppc;
-}
-
 /*
  * The max target pstate ratio is a 8 bit value in both PLATFORM_INFO MSR and
  * in TURBO_RATIO_LIMIT MSR, which pstate driver stores in max_pstate and
@@ -395,10 +385,7 @@ static void intel_pstate_init_acpi_perf_limits(struct cpufreq_policy *policy)
 	int ret;
 	int i;
 
-	if (hwp_active)
-		return;
-
-	if (!intel_pstate_get_ppc_enable_status())
+	if (!acpi_ppc || hwp_active)
 		return;
 
 	cpu = all_cpu_data[policy->cpu];
-- 
2.5.5

Re: [PATCH] Revert "cpufreq: intel_pstate: Enable PPC enforcement for servers"

[Rafael J. Wysocki]

On Wednesday, June 01, 2016 05:41:57 PM Srinivas Pandruvada wrote:
> This change reverts
> 'commit 2b3ec7650584 ("cpufreq: intel_pstate: Enable PPC enforcement for
> servers")'
> 
> Intel P State uses max P-State as the max turbo P-State. This max P-State
> can be limited by ACPI _PSS table entry 0. After
> 'commit 9522a2ff9cde ("cpufreq: intel_pstate: Enforce _PPC limits")''
> the _PSS table entry[0] will be used to cap max performance for
> enterprise and performance server by default.
> 
> Even though this is correct processing, but when the performance results
> are compared with the version before the above commit, then obviously the
> results will be worse, if the _PSS table entry 0 is not the max turbo
> P-State.
> 
> So to minimize impact on performance, this revert will disable the default
> enforcement of ACPI _PSS and ACPI _PPC. So this feature can only only be
> explicitly activated by kernel command line
> intel_pstate=support_acpi_ppc
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Thanks for the revert, but I'm wondering if we can explore one more option.

Namely, if _PSS doesn't contain the turbo state, but we know the turbo range is
there from the CPU, we can still use it and interpret _PPC for the max state as
a permission to go into the turbo range.

What do you think?

Thanks,
Rafael

Re: [PATCH] Revert "cpufreq: intel_pstate: Enable PPC enforcement for servers"

[Srinivas Pandruvada]

On Fri, 2016-06-03 at 00:00 +0200, Rafael J. Wysocki wrote:
> On Wednesday, June 01, 2016 05:41:57 PM Srinivas Pandruvada wrote:
> > 
> > This change reverts
> > 'commit 2b3ec7650584 ("cpufreq: intel_pstate: Enable PPC
> > enforcement for
> > servers")'
> > 
> > Intel P State uses max P-State as the max turbo P-State. This max
> > P-State
> > can be limited by ACPI _PSS table entry 0. After
> > 'commit 9522a2ff9cde ("cpufreq: intel_pstate: Enforce _PPC
> > limits")''
> > the _PSS table entry[0] will be used to cap max performance for
> > enterprise and performance server by default.
> > 
> > Even though this is correct processing, but when the performance
> > results
> > are compared with the version before the above commit, then
> > obviously the
> > results will be worse, if the _PSS table entry 0 is not the max
> > turbo
> > P-State.
> > 
> > So to minimize impact on performance, this revert will disable the
> > default
> > enforcement of ACPI _PSS and ACPI _PPC. So this feature can only
> > only be
> > explicitly activated by kernel command line
> > intel_pstate=support_acpi_ppc
> > 
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
> > .com>
> Thanks for the revert, but I'm wondering if we can explore one more
> option.
> 
> Namely, if _PSS doesn't contain the turbo state, but we know the
> turbo range is
> there from the CPU, we can still use it and interpret _PPC for the
> max state as
> a permission to go into the turbo range.
> 
> What do you think?
Looks like a good idea. I will experiment. What is your deadline for
-rc2 fixes?
It will give me more time to experiment and test.

Thanks,
Srinivas

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

Re: [PATCH] Revert "cpufreq: intel_pstate: Enable PPC enforcement for servers"

[Rafael J. Wysocki]

On Fri, Jun 3, 2016 at 12:05 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> On Fri, 2016-06-03 at 00:00 +0200, Rafael J. Wysocki wrote:
>> On Wednesday, June 01, 2016 05:41:57 PM Srinivas Pandruvada wrote:
>> >
>> > This change reverts
>> > 'commit 2b3ec7650584 ("cpufreq: intel_pstate: Enable PPC
>> > enforcement for
>> > servers")'
>> >
>> > Intel P State uses max P-State as the max turbo P-State. This max
>> > P-State
>> > can be limited by ACPI _PSS table entry 0. After
>> > 'commit 9522a2ff9cde ("cpufreq: intel_pstate: Enforce _PPC
>> > limits")''
>> > the _PSS table entry[0] will be used to cap max performance for
>> > enterprise and performance server by default.
>> >
>> > Even though this is correct processing, but when the performance
>> > results
>> > are compared with the version before the above commit, then
>> > obviously the
>> > results will be worse, if the _PSS table entry 0 is not the max
>> > turbo
>> > P-State.
>> >
>> > So to minimize impact on performance, this revert will disable the
>> > default
>> > enforcement of ACPI _PSS and ACPI _PPC. So this feature can only
>> > only be
>> > explicitly activated by kernel command line
>> > intel_pstate=support_acpi_ppc
>> >
>> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
>> > .com>
>> Thanks for the revert, but I'm wondering if we can explore one more
>> option.
>>
>> Namely, if _PSS doesn't contain the turbo state, but we know the
>> turbo range is
>> there from the CPU, we can still use it and interpret _PPC for the
>> max state as
>> a permission to go into the turbo range.
>>
>> What do you think?
> Looks like a good idea. I will experiment. What is your deadline for
> -rc2 fixes?
> It will give me more time to experiment and test.

That need not be -rc2 as far as I'm concerned.  Reverts are the last
resort IMO, so please take your time.