[PATCH 0/4] cpufreq governors and Intel P state driver compatibility

[PATCH 0/4] cpufreq governors and Intel P state driver compatibility

[Srinivas Pandruvada]

Intel P State driver implements two policies, performance and powersave.
The powersave policy is similar to ondemand cpufreq governor when using
acpi-cpufreq. This causes lots of confusion among users. This results
in invalid comparison of performance when acpi-cpufreq and Intel P state
performance is compared.

The reason Intel P state called powersave when it actually implemented
ondemand style P State selection, because the cpufreq core only allows
two generic policies "performance and powersave" for drivers which has
setpolicy() interface. All drivers using this interface are forced to
support these two policies.

This patchset adds feature to have configurable generic policies and
allows ondemand as one of the policy. With this approach, Intel P state
now adds support for ondemand policy and power save policy both in
addition to performance.

To be done:
cpufreq and Intel P state documentation update:
Once this approach is OK, will submit a patch to update.

Srinivas Pandruvada (4):
  cpufreq: Add configurable generic policies
  cpufreq: Add ondemand as a generic policy
  cpufreq: intel_pstate: Change powersave to ondemand policy
  cpufreq: intel_pstate: Add powersave policy support

 drivers/cpufreq/cpufreq.c      | 22 +++++++++++++-
 drivers/cpufreq/intel_pstate.c | 66 +++++++++++++++++++++++++++++++++---------
 include/linux/cpufreq.h        |  6 +++-
 3 files changed, 79 insertions(+), 15 deletions(-)

-- 
2.4.3

[PATCH 1/4] cpufreq: Add configurable generic policies

[Srinivas Pandruvada]

For drivers using cpufreq_driver->setpolicy callback, by default two
policies are available (performance and powersave). The client drivers
can't change them, even if there is no support for a policy.
This change adds a new callback get_available_policies() which provides,
a mask of supported policies. If there is no callback then only two
polices are supported "performance powersave" matching current
implementation.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/cpufreq.c | 14 +++++++++++++-
 include/linux/cpufreq.h   |  3 +++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 8412ce5..6fc6e39d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -688,7 +688,10 @@ static ssize_t show_scaling_available_governors(struct cpufreq_policy *policy,
 	struct cpufreq_governor *t;
 
 	if (!has_target()) {
-		i += sprintf(buf, "performance powersave");
+		if (policy->available_policies & CPUFREQ_POLICY_PERFORMANCE)
+			i += sprintf(buf, "performance ");
+		if (policy->available_policies & CPUFREQ_POLICY_POWERSAVE)
+			i += sprintf(&buf[i], "powersave ");
 		goto out;
 	}
 
@@ -963,9 +966,18 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
 {
 	struct cpufreq_governor *gov = NULL;
 	struct cpufreq_policy new_policy;
+	u8 mask;
 
 	memcpy(&new_policy, policy, sizeof(*policy));
 
+	if (cpufreq_driver->get_available_policies) {
+		if (!cpufreq_driver->get_available_policies(&mask))
+			policy->available_policies = mask;
+	}
+	if (!policy->available_policies)
+		policy->available_policies = CPUFREQ_POLICY_PERFORMANCE |
+						CPUFREQ_POLICY_POWERSAVE;
+
 	/* Update governor of new_policy to the governor used before hotplug */
 	gov = find_governor(policy->last_governor);
 	if (gov)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 177c768..8259c3c 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -76,6 +76,7 @@ struct cpufreq_policy {
 	unsigned int		restore_freq; /* = policy->cur before transition */
 	unsigned int		suspend_freq; /* freq to set during suspend */
 
+	u8			available_policies;
 	unsigned int		policy; /* see above */
 	unsigned int		last_policy; /* policy before unplug */
 	struct cpufreq_governor	*governor; /* see below */
@@ -229,6 +230,8 @@ struct cpufreq_driver {
 	int		(*init)(struct cpufreq_policy *policy);
 	int		(*verify)(struct cpufreq_policy *policy);
 
+	/* Get available generic policies */
+	int		(*get_available_policies)(u8 *mask);
 	/* define one out of two */
 	int		(*setpolicy)(struct cpufreq_policy *policy);
 
-- 
2.4.3

Re: [PATCH 1/4] cpufreq: Add configurable generic policies

[Viresh Kumar]

On 04-12-15, 16:08, Srinivas Pandruvada wrote:
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 177c768..8259c3c 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -76,6 +76,7 @@ struct cpufreq_policy {
>  	unsigned int		restore_freq; /* = policy->cur before transition */
>  	unsigned int		suspend_freq; /* freq to set during suspend */
>  
> +	u8			available_policies;

Why don't we let the set-policy drivers fill in this field? No
callback required then.

-- 
viresh

Re: [PATCH 1/4] cpufreq: Add configurable generic policies

[Srinivas Pandruvada]

On Mon, 2015-12-07 at 15:03 +0530, Viresh Kumar wrote:
> On 04-12-15, 16:08, Srinivas Pandruvada wrote:
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index 177c768..8259c3c 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -76,6 +76,7 @@ struct cpufreq_policy {
> >  	unsigned int		restore_freq; /* = policy->cur
> > before transition */
> >  	unsigned int		suspend_freq; /* freq to set
> > during suspend */
> >  
> > +	u8			available_policies;

> Why don't we let the set-policy drivers fill in this field? No
> callback required then.
> 
That is fine with me. In next revision I will add that.

Thanks,
Srinivas

[PATCH 2/4] cpufreq: Add ondemand as a generic policy

[Srinivas Pandruvada]

Allow ondemand policy as a supported generic policy. Client drivers can
enable this policy by using get_available_policies() callback.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/cpufreq.c | 8 ++++++++
 include/linux/cpufreq.h   | 3 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6fc6e39d..86579fa 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -528,6 +528,10 @@ static int cpufreq_parse_governor(char *str_governor, unsigned int *policy,
 						CPUFREQ_NAME_LEN)) {
 			*policy = CPUFREQ_POLICY_POWERSAVE;
 			err = 0;
+		} else if (!strncasecmp(str_governor, "ondemand",
+					CPUFREQ_NAME_LEN)) {
+			*policy = CPUFREQ_POLICY_ONDEMAND;
+			err = 0;
 		}
 	} else {
 		struct cpufreq_governor *t;
@@ -640,6 +644,8 @@ static ssize_t show_scaling_governor(struct cpufreq_policy *policy, char *buf)
 		return sprintf(buf, "powersave\n");
 	else if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
 		return sprintf(buf, "performance\n");
+	else if (policy->policy == CPUFREQ_POLICY_ONDEMAND)
+		return sprintf(buf, "ondemand\n");
 	else if (policy->governor)
 		return scnprintf(buf, CPUFREQ_NAME_PLEN, "%s\n",
 				policy->governor->name);
@@ -692,6 +698,8 @@ static ssize_t show_scaling_available_governors(struct cpufreq_policy *policy,
 			i += sprintf(buf, "performance ");
 		if (policy->available_policies & CPUFREQ_POLICY_POWERSAVE)
 			i += sprintf(&buf[i], "powersave ");
+		if (policy->available_policies & CPUFREQ_POLICY_ONDEMAND)
+			i += sprintf(&buf[i], "ondemand ");
 		goto out;
 	}
 
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 8259c3c..65f85d5 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -232,7 +232,7 @@ struct cpufreq_driver {
 
 	/* Get available generic policies */
 	int		(*get_available_policies)(u8 *mask);
-	/* define one out of two */
+	/* define one out of three */
 	int		(*setpolicy)(struct cpufreq_policy *policy);
 
 	/*
@@ -434,6 +434,7 @@ static inline unsigned long cpufreq_scale(unsigned long old, u_int div,
  */
 #define CPUFREQ_POLICY_POWERSAVE	(1)
 #define CPUFREQ_POLICY_PERFORMANCE	(2)
+#define CPUFREQ_POLICY_ONDEMAND		(4)
 
 /* Governor Events */
 #define CPUFREQ_GOV_START	1
-- 
2.4.3

Re: [PATCH 2/4] cpufreq: Add ondemand as a generic policy

[Viresh Kumar]

On 04-12-15, 16:08, Srinivas Pandruvada wrote:
> Allow ondemand policy as a supported generic policy. Client drivers can
> enable this policy by using get_available_policies() callback.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/cpufreq/cpufreq.c | 8 ++++++++
>  include/linux/cpufreq.h   | 3 ++-
>  2 files changed, 10 insertions(+), 1 deletion(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH 3/4] cpufreq: intel_pstate: Change powersave to ondemand policy

[Srinivas Pandruvada]

Move the current Intel powersave policy processing to ondemand policy.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index fb92402..61b7118 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -176,7 +176,7 @@ static struct perf_limits performance_limits = {
 	.min_sysfs_pct = 0,
 };
 
-static struct perf_limits powersave_limits = {
+static struct perf_limits ondemand_limits = {
 	.no_turbo = 0,
 	.turbo_disabled = 0,
 	.max_perf_pct = 100,
@@ -192,7 +192,7 @@ static struct perf_limits powersave_limits = {
 #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE
 static struct perf_limits *limits = &performance_limits;
 #else
-static struct perf_limits *limits = &powersave_limits;
+static struct perf_limits *limits = &ondemand_limits;
 #endif
 
 static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
@@ -1143,8 +1143,8 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
 		return 0;
 	}
 
-	pr_debug("intel_pstate: set powersave\n");
-	limits = &powersave_limits;
+	pr_debug("intel_pstate: set ondemand\n");
+	limits = &ondemand_limits;
 	limits->min_policy_pct = (policy->min * 100) / policy->cpuinfo.max_freq;
 	limits->min_policy_pct = clamp_t(int, limits->min_policy_pct, 0 , 100);
 	limits->max_policy_pct = DIV_ROUND_UP(policy->max * 100,
@@ -1180,7 +1180,7 @@ static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
 {
 	cpufreq_verify_within_cpu_limits(policy);
 
-	if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
+	if (policy->policy != CPUFREQ_POLICY_ONDEMAND &&
 	    policy->policy != CPUFREQ_POLICY_PERFORMANCE)
 		return -EINVAL;
 
@@ -1215,7 +1215,7 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 	if (limits->min_perf_pct == 100 && limits->max_perf_pct == 100)
 		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
 	else
-		policy->policy = CPUFREQ_POLICY_POWERSAVE;
+		policy->policy = CPUFREQ_POLICY_ONDEMAND;
 
 	policy->min = cpu->pstate.min_pstate * cpu->pstate.scaling;
 	policy->max = cpu->pstate.turbo_pstate * cpu->pstate.scaling;
@@ -1230,10 +1230,19 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 	return 0;
 }
 
+static int intel_pstate_available_policies(u8 *mask)
+{
+
+	*mask = CPUFREQ_POLICY_PERFORMANCE | CPUFREQ_POLICY_ONDEMAND;
+
+	return 0;
+}
+
 static struct cpufreq_driver intel_pstate_driver = {
 	.flags		= CPUFREQ_CONST_LOOPS,
 	.verify		= intel_pstate_verify_policy,
 	.setpolicy	= intel_pstate_set_policy,
+	.get_available_policies = intel_pstate_available_policies,
 	.get		= intel_pstate_get,
 	.init		= intel_pstate_cpu_init,
 	.stop_cpu	= intel_pstate_stop_cpu,
-- 
2.4.3

[PATCH 4/4] cpufreq: intel_pstate: Add powersave policy support

[Srinivas Pandruvada]

Implement support for powersave policy. The powersave policy will limit
the max performance to P-State = Maximum Efficiency Ratio.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 49 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 61b7118..417d020 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -189,8 +189,23 @@ static struct perf_limits ondemand_limits = {
 	.min_sysfs_pct = 0,
 };
 
-#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE
+static struct perf_limits powersave_limits = {
+	.no_turbo = 1,
+	.turbo_disabled = 1,
+	.max_perf_pct = 0,
+	.max_perf = int_tofp(1),
+	.min_perf_pct = 0,
+	.min_perf = int_tofp(1),
+	.max_policy_pct = 0,
+	.max_sysfs_pct = 0,
+	.min_policy_pct = 0,
+	.min_sysfs_pct = 0,
+};
+
+#if defined(CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE)
 static struct perf_limits *limits = &performance_limits;
+#elif defined(CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE)
+static struct perf_limits *limits = &powersave_limits;
 #else
 static struct perf_limits *limits = &ondemand_limits;
 #endif
@@ -1143,13 +1158,26 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
 		return 0;
 	}
 
-	pr_debug("intel_pstate: set ondemand\n");
-	limits = &ondemand_limits;
-	limits->min_policy_pct = (policy->min * 100) / policy->cpuinfo.max_freq;
-	limits->min_policy_pct = clamp_t(int, limits->min_policy_pct, 0 , 100);
-	limits->max_policy_pct = DIV_ROUND_UP(policy->max * 100,
-					      policy->cpuinfo.max_freq);
-	limits->max_policy_pct = clamp_t(int, limits->max_policy_pct, 0 , 100);
+	if (policy->policy == CPUFREQ_POLICY_POWERSAVE) {
+		pr_debug("intel_pstate: set powersave\n");
+		limits = &powersave_limits;
+		limits->min_policy_pct = (policy->cpuinfo.min_freq * 100) /
+					  policy->cpuinfo.max_freq;
+		limits->min_policy_pct = clamp_t(int, limits->min_policy_pct,
+						 0, 100);
+		limits->max_policy_pct = limits->min_policy_pct;
+	} else {
+		pr_debug("intel_pstate: set ondemand\n");
+		limits = &ondemand_limits;
+		limits->min_policy_pct = (policy->min * 100) /
+					  policy->cpuinfo.max_freq;
+		limits->min_policy_pct = clamp_t(int, limits->min_policy_pct,
+						 0, 100);
+		limits->max_policy_pct = DIV_ROUND_UP(policy->max * 100,
+						policy->cpuinfo.max_freq);
+		limits->max_policy_pct = clamp_t(int, limits->max_policy_pct,
+						 0, 100);
+	}
 
 	/* Normalize user input to [min_policy_pct, max_policy_pct] */
 	limits->min_perf_pct = max(limits->min_policy_pct,
@@ -1181,7 +1209,8 @@ static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
 	cpufreq_verify_within_cpu_limits(policy);
 
 	if (policy->policy != CPUFREQ_POLICY_ONDEMAND &&
-	    policy->policy != CPUFREQ_POLICY_PERFORMANCE)
+	    policy->policy != CPUFREQ_POLICY_PERFORMANCE &&
+	    policy->policy != CPUFREQ_POLICY_POWERSAVE)
 		return -EINVAL;
 
 	return 0;
@@ -1234,6 +1263,8 @@ static int intel_pstate_available_policies(u8 *mask)
 {
 
 	*mask = CPUFREQ_POLICY_PERFORMANCE | CPUFREQ_POLICY_ONDEMAND;
+	if (!hwp_active)
+		*mask |= CPUFREQ_POLICY_POWERSAVE;
 
 	return 0;
 }
-- 
2.4.3

Re: [PATCH 0/4] cpufreq governors and Intel P state driver compatibility

[Thomas Renninger]

On Friday, December 04, 2015 04:08:34 PM Srinivas Pandruvada wrote:
> Intel P State driver implements two policies, performance and powersave.
> The powersave policy is similar to ondemand cpufreq governor when using
> acpi-cpufreq. This causes lots of confusion among users. This results
> in invalid comparison of performance when acpi-cpufreq and Intel P state
> performance is compared.

After several years you want to change this again?

We released documentation for this for SLE 12 recently.
It was not that easy to phrase, but it would be wrong again with newer
kernels..., sigh.

But yeah, maybe this change should be finally made.
One idea, not sure it is better: Revert the powersave governor. Then it
is clear on which implemenation of intel_pstate you are running.
And you do not accidentially think you are adjusting freqs on powersave
governor but in real you are running fixed at lowest freq.

When you are looking at cleaning up intel_pstate driver here a
request/question from me on the cpufreq list from March 2014.
>From what I see, this is still valid and these pstate specific sysfs
knobs exist as general cpufreq files already. So the pstate ones should
vanish:

1) sysfs tunables:
   - max_perf_pct, min_perf_pct
     According to Documentation/cpu-freq/intel-pstate.txt this is:
      max_perf_pct: limits the maximum P state that will be requested by
      the driver stated as a percentage of the available performance.

      min_perf_pct: limits the minimum P state that will be  requested by
      the driver stated as a percentage of the available performance.

     Why is this needed, there already is:
     scaling_max_freq, scaling_min_freq

     How are both connected?
     For me those tunable are doing the same and intel_pstate specific ones
     should vanish to have one cpufreq min/max frequency interface exported
     to userspace on all archs/cpufreq drivers.

   - no_turbo: limits the driver to selecting P states below the turbo
     frequency range.

     Again, there is the general cpufreq "boost" tunable defined in cpufreq.c:
     ssize_t show_boost(..)
     static ssize_t store_boost(...)
     define_one_global_rw(boost);

     What is the difference, why does intel-pstate need its own tunable?

    Thomas

Re: [PATCH 0/4] cpufreq governors and Intel P state driver compatibility

[Srinivas Pandruvada]

On Tue, 2015-12-08 at 15:35 +0100, Thomas Renninger wrote:
> On Friday, December 04, 2015 04:08:34 PM Srinivas Pandruvada wrote:
> > Intel P State driver implements two policies, performance and powersave.
> > The powersave policy is similar to ondemand cpufreq governor when using
> > acpi-cpufreq. This causes lots of confusion among users. This results
> > in invalid comparison of performance when acpi-cpufreq and Intel P state
> > performance is compared.
> 
> After several years you want to change this again?
This is based on feedback. But again, if this breaks some users, then we
need to think.
> 
> We released documentation for this for SLE 12 recently.
> It was not that easy to phrase, but it would be wrong again with newer
> kernels..., sigh.
Will this patchset break SLE 12?
Are you defaulting to "powersave" policy of Intel P state?
> 
> But yeah, maybe this change should be finally made.
> One idea, not sure it is better: Revert the powersave governor. Then it
> is clear on which implemenation of intel_pstate you are running.
> And you do not accidentially think you are adjusting freqs on powersave
> governor but in real you are running fixed at lowest freq.
So If we don't implement powersave policy ("running at lowest
frequency") and publish "performance ondemand". Unless userspace is not
explicitly setting "performance" or using CONFIG_DEFAULT_PERF.., we will
always use "ondemand", so customers won't notice any difference. Is this
OK?

Also question: Do you see issue in calling "ondemand" policy?

> 
> When you are looking at cleaning up intel_pstate driver here a
> request/question from me on the cpufreq list from March 2014.
> From what I see, this is still valid and these pstate specific sysfs
> knobs exist as general cpufreq files already. So the pstate ones should
> vanish:
> 
There is some history to this and certification tests are relying on
this. So I am planning to better document these as first step.
The idea is to move away from the concept of a frequency to a P state,
which is an operating point (frequency and voltage pair). Nothing stops
having two P states with same frequency but different voltage point.
> 1) sysfs tunables:
>    - max_perf_pct, min_perf_pct
>      According to Documentation/cpu-freq/intel-pstate.txt this is:
>       max_perf_pct: limits the maximum P state that will be requested by
>       the driver stated as a percentage of the available performance.
> 
>       min_perf_pct: limits the minimum P state that will be  requested by
>       the driver stated as a percentage of the available performance.
> 
>      Why is this needed, there already is:
>      scaling_max_freq, scaling_min_freq
This is not enough to show which range out of this belongs to turbo
region and which not. The Intel P state tunable have extra attributes to
know this. Since the range is opportunistic, it is not good to
explicitly publish using scaling_available_frequencies.
> 
>      How are both connected?
>      For me those tunable are doing the same and intel_pstate specific ones
>      should vanish to have one cpufreq min/max frequency interface exported
>      to userspace on all archs/cpufreq drivers.
> 
>    - no_turbo: limits the driver to selecting P states below the turbo
>      frequency range.
> 
>      Again, there is the general cpufreq "boost" tunable defined in cpufreq.c:
>      ssize_t show_boost(..)
>      static ssize_t store_boost(...)
>      define_one_global_rw(boost);
> 
>      What is the difference, why does intel-pstate need its own tunable?
> 
Thanks,
Srinivas

>     Thomas
> --
> 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 0/4] cpufreq governors and Intel P state driver compatibility

[Thomas Renninger]

On Tuesday, December 08, 2015 09:43:03 AM Srinivas Pandruvada wrote:
> On Tue, 2015-12-08 at 15:35 +0100, Thomas Renninger wrote:
> > On Friday, December 04, 2015 04:08:34 PM Srinivas Pandruvada wrote:
> > > Intel P State driver implements two policies, performance and powersave.
> > > The powersave policy is similar to ondemand cpufreq governor when using
> > > acpi-cpufreq. This causes lots of confusion among users. This results
> > > in invalid comparison of performance when acpi-cpufreq and Intel P state
> > > performance is compared.
> > 
> > After several years you want to change this again?
> 
> This is based on feedback. But again, if this breaks some users, then we
> need to think.
> 
> > We released documentation for this for SLE 12 recently.
> > It was not that easy to phrase, but it would be wrong again with newer
> > kernels..., sigh.
> 
> Will this patchset break SLE 12?
> Are you defaulting to "powersave" policy of Intel P state?

We use powersave as default with intel_pstate because this is default.

You won't "break" SLE 12 and I mostly thought about documentation.
This is the intel_pstate part:

-------------------------------------
Not all drivers use the in-kernel governors to dynamically scale power 
frequency at runtime. For example, the intel_pstate driver adjusts power 
frequency itself. Use the cpupower frequency-info command to find out which 
driver your system uses.
-------------------------------------

Fortunately we cut out the part to explain why "powersave" governor shows
up and what it does. So we are more or less safe.

Still.., instead of providing the next quick shot, we may want to think
further...
Having an ondemand governor and a faked may lead to more confusion in the
future.

In general, worst that can happen for SLE (probably same for RHEL) are any 
kind of performance regressions that are introduced by trying to save more 
power (on the same HW).
Customers/users will find them and complain and need at least a param to
get back to old (power wasting) behaviour.

   Thomas

Re: [PATCH 0/4] cpufreq governors and Intel P state driver compatibility

[Srinivas Pandruvada]

On Wed, 2015-12-09 at 16:02 +0100, Thomas Renninger wrote:
> On Tuesday, December 08, 2015 09:43:03 AM Srinivas Pandruvada wrote:
> > On Tue, 2015-12-08 at 15:35 +0100, Thomas Renninger wrote:
> > > On Friday, December 04, 2015 04:08:34 PM Srinivas Pandruvada
> > > wrote:
> > > > Intel P State driver implements two policies, performance and
> > > > powersave.
> > > > The powersave policy is similar to ondemand cpufreq governor
> > > > when using
> > > > acpi-cpufreq. This causes lots of confusion among users. This
> > > > results
> > > > in invalid comparison of performance when acpi-cpufreq and
> > > > Intel P state
> > > > performance is compared.
> > > 
> > > After several years you want to change this again?
> > 
> > This is based on feedback. But again, if this breaks some users,
> > then we
> > need to think.
> > 
> > > We released documentation for this for SLE 12 recently.
> > > It was not that easy to phrase, but it would be wrong again with
> > > newer
> > > kernels..., sigh.
> > 
> > Will this patchset break SLE 12?
> > Are you defaulting to "powersave" policy of Intel P state?
> 
> We use powersave as default with intel_pstate because this is
> default.
> 
> You won't "break" SLE 12 and I mostly thought about documentation.
> This is the intel_pstate part:
> 
> -------------------------------------
> Not all drivers use the in-kernel governors to dynamically scale
> power 
> frequency at runtime. For example, the intel_pstate driver adjusts
> power 
> frequency itself. Use the cpupower frequency-info command to find out
> which 
> driver your system uses.
> -------------------------------------
> 
> Fortunately we cut out the part to explain why "powersave" governor
> shows
> up and what it does. So we are more or less safe.
> 
> Still.., instead of providing the next quick shot, we may want to
> think
> further...
> Having an ondemand governor and a faked may lead to more confusion in
> the
> future.

May be calling "intel_pstate_ondemand" instead of powersave. But that
needs lots of changes in cpufreq core or we have to implement this
driver as a governor + cpufreq driver with target() I/F like acpi
-cpufreq.


> In general, worst that can happen for SLE (probably same for RHEL)
> are any 
> kind of performance regressions that are introduced by trying to save
> more 
> power (on the same HW).
> Customers/users will find them and complain and need at least a param
> to
> get back to old (power wasting) behaviour.
> 
I didn't get this point.

Thanks,
Srinivas

>    Thomas
>