[PATCH 0/5] intel_pstate updates for v3.14-rcX

[PATCH 0/5] intel_pstate updates for v3.14-rcX

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Based on v3.14-rc2

Patch 1 removes energy reporting the patch from Maurizio Lambardi 
 intel_pstate: fix race condition in intel_pstate_init() can be dropped.

Patch 2-3 cleanups

Patch 4-5 Baytrail bug fixes

Dirk Brandewie (5):
  intel_pstate: Remove energy reporting from pstate_sample tracepoint
  intel_pstate: remove unneeded sample buffers
  intel_pstate: fix pid_reset to use fixed point values
  intel_pstate: Use LFM bus ratio as min ratio/P state
  intel_pstate: Add support for Baytrail turbo P states

 drivers/cpufreq/intel_pstate.c | 53 +++++++++++++++++++++---------------------
 include/trace/events/power.h   |  7 +-----
 2 files changed, 27 insertions(+), 33 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] intel_pstate: Remove energy reporting from pstate_sample tracepoint

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Remove the reporting of energy since it does not provide any useful
information about the state of the driver and will be a maintainance
headache going forward since the RAPL energy units register is not
architectural and subject to change between micro-architectures

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 9 ---------
 include/trace/events/power.h   | 7 +------
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 79606f4..c788abf 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -51,8 +51,6 @@ static inline int32_t div_fp(int32_t x, int32_t y)
 	return div_s64((int64_t)x << FRAC_BITS, (int64_t)y);
 }
 
-static u64 energy_divisor;
-
 struct sample {
 	int32_t core_pct_busy;
 	u64 aperf;
@@ -630,12 +628,10 @@ static void intel_pstate_timer_func(unsigned long __data)
 {
 	struct cpudata *cpu = (struct cpudata *) __data;
 	struct sample *sample;
-	u64 energy;
 
 	intel_pstate_sample(cpu);
 
 	sample = &cpu->samples[cpu->sample_ptr];
-	rdmsrl(MSR_PKG_ENERGY_STATUS, energy);
 
 	intel_pstate_adjust_busy_pstate(cpu);
 
@@ -644,7 +640,6 @@ static void intel_pstate_timer_func(unsigned long __data)
 			cpu->pstate.current_pstate,
 			sample->mperf,
 			sample->aperf,
-			div64_u64(energy, energy_divisor),
 			sample->freq);
 
 	intel_pstate_set_sample_time(cpu);
@@ -926,7 +921,6 @@ static int __init intel_pstate_init(void)
 	int cpu, rc = 0;
 	const struct x86_cpu_id *id;
 	struct cpu_defaults *cpu_info;
-	u64 units;
 
 	if (no_load)
 		return -ENODEV;
@@ -960,9 +954,6 @@ static int __init intel_pstate_init(void)
 	if (rc)
 		goto out;
 
-	rdmsrl(MSR_RAPL_POWER_UNIT, units);
-	energy_divisor = 1 << ((units >> 8) & 0x1f); /* bits{12:8} */
-
 	intel_pstate_debug_expose_params();
 	intel_pstate_sysfs_expose_params();
 
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index 9e9475c..e5bf9a7 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -42,7 +42,6 @@ TRACE_EVENT(pstate_sample,
 		u32 state,
 		u64 mperf,
 		u64 aperf,
-		u32 energy,
 		u32 freq
 		),
 
@@ -51,7 +50,6 @@ TRACE_EVENT(pstate_sample,
 		state,
 		mperf,
 		aperf,
-		energy,
 		freq
 		),
 
@@ -61,7 +59,6 @@ TRACE_EVENT(pstate_sample,
 		__field(u32, state)
 		__field(u64, mperf)
 		__field(u64, aperf)
-		__field(u32, energy)
 		__field(u32, freq)
 
 	),
@@ -72,17 +69,15 @@ TRACE_EVENT(pstate_sample,
 		__entry->state = state;
 		__entry->mperf = mperf;
 		__entry->aperf = aperf;
-		__entry->energy = energy;
 		__entry->freq = freq;
 		),
 
-	TP_printk("core_busy=%lu scaled=%lu state=%lu mperf=%llu aperf=%llu energy=%lu freq=%lu ",
+	TP_printk("core_busy=%lu scaled=%lu state=%lu mperf=%llu aperf=%llu freq=%lu ",
 		(unsigned long)__entry->core_busy,
 		(unsigned long)__entry->scaled_busy,
 		(unsigned long)__entry->state,
 		(unsigned long long)__entry->mperf,
 		(unsigned long long)__entry->aperf,
-		(unsigned long)__entry->energy,
 		(unsigned long)__entry->freq
 		)
 
-- 
1.8.3.1

[PATCH 2/5] intel_pstate: remove unneeded sample buffers

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Remove unneeded sample buffers, intel_pstate operates on the most
recent sample only.  This save some memory and make the code more
readable.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index c788abf..d692b10 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -96,8 +96,7 @@ struct cpudata {
 	u64	prev_aperf;
 	u64	prev_mperf;
 	unsigned long long prev_tsc;
-	int	sample_ptr;
-	struct sample samples[SAMPLE_COUNT];
+	struct sample sample;
 };
 
 static struct cpudata **all_cpu_data;
@@ -570,15 +569,15 @@ static inline void intel_pstate_sample(struct cpudata *cpu)
 	rdmsrl(MSR_IA32_MPERF, mperf);
 	tsc = native_read_tsc();
 
-	cpu->sample_ptr = (cpu->sample_ptr + 1) % SAMPLE_COUNT;
-	cpu->samples[cpu->sample_ptr].aperf = aperf;
-	cpu->samples[cpu->sample_ptr].mperf = mperf;
-	cpu->samples[cpu->sample_ptr].tsc = tsc;
-	cpu->samples[cpu->sample_ptr].aperf -= cpu->prev_aperf;
-	cpu->samples[cpu->sample_ptr].mperf -= cpu->prev_mperf;
-	cpu->samples[cpu->sample_ptr].tsc -= cpu->prev_tsc;
 
-	intel_pstate_calc_busy(cpu, &cpu->samples[cpu->sample_ptr]);
+	cpu->sample.aperf = aperf;
+	cpu->sample.mperf = mperf;
+	cpu->sample.tsc = tsc;
+	cpu->sample.aperf -= cpu->prev_aperf;
+	cpu->sample.mperf -= cpu->prev_mperf;
+	cpu->sample.tsc -= cpu->prev_tsc;
+
+	intel_pstate_calc_busy(cpu, &cpu->sample);
 
 	cpu->prev_aperf = aperf;
 	cpu->prev_mperf = mperf;
@@ -598,7 +597,7 @@ static inline int32_t intel_pstate_get_scaled_busy(struct cpudata *cpu)
 {
 	int32_t core_busy, max_pstate, current_pstate;
 
-	core_busy = cpu->samples[cpu->sample_ptr].core_pct_busy;
+	core_busy = cpu->sample.core_pct_busy;
 	max_pstate = int_tofp(cpu->pstate.max_pstate);
 	current_pstate = int_tofp(cpu->pstate.current_pstate);
 	return mul_fp(core_busy, div_fp(max_pstate, current_pstate));
@@ -631,7 +630,7 @@ static void intel_pstate_timer_func(unsigned long __data)
 
 	intel_pstate_sample(cpu);
 
-	sample = &cpu->samples[cpu->sample_ptr];
+	sample = &cpu->sample;
 
 	intel_pstate_adjust_busy_pstate(cpu);
 
@@ -712,7 +711,7 @@ static unsigned int intel_pstate_get(unsigned int cpu_num)
 	cpu = all_cpu_data[cpu_num];
 	if (!cpu)
 		return 0;
-	sample = &cpu->samples[cpu->sample_ptr];
+	sample = &cpu->sample;
 	return sample->freq;
 }
 
-- 
1.8.3.1

[PATCH 3/5] intel_pstate: fix pid_reset to use fixed point values

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

commit d253d2a526 Improve accuracy by not truncating until final
result, changed internal variables of the PID to be fixed point
numbers. Update the pid_reset() to reflect this change.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@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 d692b10..b80c03d 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -150,7 +150,7 @@ static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
 	pid->setpoint = setpoint;
 	pid->deadband  = deadband;
 	pid->integral  = int_tofp(integral);
-	pid->last_err  = setpoint - busy;
+	pid->last_err  = int_tofp(setpoint) - int_tofp(busy);
 }
 
 static inline void pid_p_gain_set(struct _pid *pid, int percent)
-- 
1.8.3.1

[PATCH 4/5] intel_pstate: Use LFM bus ratio as min ratio/P state

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Using LFM ratio provides better perf/watt vs min ratio.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@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 b80c03d..ff45ce1 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -356,7 +356,7 @@ static int byt_get_min_pstate(void)
 {
 	u64 value;
 	rdmsrl(BYT_RATIOS, value);
-	return value & 0xFF;
+	return (value >> 8) & 0xFF;
 }
 
 static int byt_get_max_pstate(void)
-- 
1.8.3.1

[PATCH 5/5] intel_pstate: Add support for Baytrail turbo P states

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

A documentation update exposed the existance of the turbo ratio
register. Update baytrail support to use the turbo range.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index ff45ce1..ea58eb6 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -34,8 +34,10 @@
 
 #define SAMPLE_COUNT		3
 
-#define BYT_RATIOS	0x66a
-#define BYT_VIDS        0x66b
+#define BYT_RATIOS		0x66a
+#define BYT_VIDS		0x66b
+#define BYT_TURBO_RATIOS	0x66c
+
 
 #define FRAC_BITS 8
 #define int_tofp(X) ((int64_t)(X) << FRAC_BITS)
@@ -366,6 +368,13 @@ static int byt_get_max_pstate(void)
 	return (value >> 16) & 0xFF;
 }
 
+static int byt_get_turbo_pstate(void)
+{
+	u64 value;
+	rdmsrl(BYT_TURBO_RATIOS, value);
+	return value & 0x3F;
+}
+
 static void byt_set_pstate(struct cpudata *cpudata, int pstate)
 {
 	u64 val;
@@ -468,7 +477,7 @@ static struct cpu_defaults byt_params = {
 	.funcs = {
 		.get_max = byt_get_max_pstate,
 		.get_min = byt_get_min_pstate,
-		.get_turbo = byt_get_max_pstate,
+		.get_turbo = byt_get_turbo_pstate,
 		.set = byt_set_pstate,
 		.get_vid = byt_get_vid,
 	},
-- 
1.8.3.1

Re: [PATCH 2/5] intel_pstate: remove unneeded sample buffers

[Li, Aubrey]

On 2014/2/13 2:01, dirk.brandewie@gmail.com wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> 
> Remove unneeded sample buffers, intel_pstate operates on the most
> recent sample only.  This save some memory and make the code more
> readable.
> 
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index c788abf..d692b10 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -96,8 +96,7 @@ struct cpudata {
>  	u64	prev_aperf;
>  	u64	prev_mperf;
>  	unsigned long long prev_tsc;
> -	int	sample_ptr;
> -	struct sample samples[SAMPLE_COUNT];
> +	struct sample sample;
>  };
>  
>  static struct cpudata **all_cpu_data;
> @@ -570,15 +569,15 @@ static inline void intel_pstate_sample(struct cpudata *cpu)
>  	rdmsrl(MSR_IA32_MPERF, mperf);
>  	tsc = native_read_tsc();

To be pikcy, just want the ratio more accurate, since aperf and mperf
are a pair, do you want to turn interrupt off to make sure nothing
occurs between reading aperf and mperf? and since native_read_tsc() is
called here, better to put into no interrupt context as well.

Thanks,
-Aubrey

>  
> -	cpu->sample_ptr = (cpu->sample_ptr + 1) % SAMPLE_COUNT;
> -	cpu->samples[cpu->sample_ptr].aperf = aperf;
> -	cpu->samples[cpu->sample_ptr].mperf = mperf;
> -	cpu->samples[cpu->sample_ptr].tsc = tsc;
> -	cpu->samples[cpu->sample_ptr].aperf -= cpu->prev_aperf;
> -	cpu->samples[cpu->sample_ptr].mperf -= cpu->prev_mperf;
> -	cpu->samples[cpu->sample_ptr].tsc -= cpu->prev_tsc;
>  
> -	intel_pstate_calc_busy(cpu, &cpu->samples[cpu->sample_ptr]);
> +	cpu->sample.aperf = aperf;
> +	cpu->sample.mperf = mperf;
> +	cpu->sample.tsc = tsc;
> +	cpu->sample.aperf -= cpu->prev_aperf;
> +	cpu->sample.mperf -= cpu->prev_mperf;
> +	cpu->sample.tsc -= cpu->prev_tsc;
> +
> +	intel_pstate_calc_busy(cpu, &cpu->sample);
>  
>  	cpu->prev_aperf = aperf;
>  	cpu->prev_mperf = mperf;
> @@ -598,7 +597,7 @@ static inline int32_t intel_pstate_get_scaled_busy(struct cpudata *cpu)
>  {
>  	int32_t core_busy, max_pstate, current_pstate;
>  
> -	core_busy = cpu->samples[cpu->sample_ptr].core_pct_busy;
> +	core_busy = cpu->sample.core_pct_busy;
>  	max_pstate = int_tofp(cpu->pstate.max_pstate);
>  	current_pstate = int_tofp(cpu->pstate.current_pstate);
>  	return mul_fp(core_busy, div_fp(max_pstate, current_pstate));
> @@ -631,7 +630,7 @@ static void intel_pstate_timer_func(unsigned long __data)
>  
>  	intel_pstate_sample(cpu);
>  
> -	sample = &cpu->samples[cpu->sample_ptr];
> +	sample = &cpu->sample;
>  
>  	intel_pstate_adjust_busy_pstate(cpu);
>  
> @@ -712,7 +711,7 @@ static unsigned int intel_pstate_get(unsigned int cpu_num)
>  	cpu = all_cpu_data[cpu_num];
>  	if (!cpu)
>  		return 0;
> -	sample = &cpu->samples[cpu->sample_ptr];
> +	sample = &cpu->sample;
>  	return sample->freq;
>  }
>  
> 

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Wednesday, February 12, 2014 10:01:02 AM dirk.brandewie@gmail.com wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> 
> Based on v3.14-rc2
> 
> Patch 1 removes energy reporting the patch from Maurizio Lambardi 
>  intel_pstate: fix race condition in intel_pstate_init() can be dropped.
> 

OK, I'll replace that patch with your [1/5].

> Patch 2-3 cleanups
> 
> Patch 4-5 Baytrail bug fixes

How [4-5/5] depend on [2-3/5]?

I'd prefer to push fixes only for 3.14 at this point to be honest.

> Dirk Brandewie (5):
>   intel_pstate: Remove energy reporting from pstate_sample tracepoint
>   intel_pstate: remove unneeded sample buffers
>   intel_pstate: fix pid_reset to use fixed point values
>   intel_pstate: Use LFM bus ratio as min ratio/P state
>   intel_pstate: Add support for Baytrail turbo P states
> 
>  drivers/cpufreq/intel_pstate.c | 53 +++++++++++++++++++++---------------------
>  include/trace/events/power.h   |  7 +-----
>  2 files changed, 27 insertions(+), 33 deletions(-)

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Thursday, February 13, 2014 01:47:43 AM Rafael J. Wysocki wrote:
> On Wednesday, February 12, 2014 10:01:02 AM dirk.brandewie@gmail.com wrote:
> > From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> > 
> > Based on v3.14-rc2
> > 
> > Patch 1 removes energy reporting the patch from Maurizio Lambardi 
> >  intel_pstate: fix race condition in intel_pstate_init() can be dropped.
> > 
> 
> OK, I'll replace that patch with your [1/5].

On a second thought, we'll need a fix for stable for that and quite frankly
your [1/5] doesn't really fit there.  So I'm going to push the Maurizio's
patch anyway and plaese rebase the whole series on top of it.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Thursday, February 13, 2014 02:00:09 AM Rafael J. Wysocki wrote:
> On Thursday, February 13, 2014 01:47:43 AM Rafael J. Wysocki wrote:
> > On Wednesday, February 12, 2014 10:01:02 AM dirk.brandewie@gmail.com wrote:
> > > From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> > > 
> > > Based on v3.14-rc2
> > > 
> > > Patch 1 removes energy reporting the patch from Maurizio Lambardi 
> > >  intel_pstate: fix race condition in intel_pstate_init() can be dropped.
> > > 
> > 
> > OK, I'll replace that patch with your [1/5].
> 
> On a second thought, we'll need a fix for stable for that and quite frankly
> your [1/5] doesn't really fit there.  So I'm going to push the Maurizio's
> patch anyway and plaese rebase the whole series on top of it.

OK, double checked, sorry for the confusion!

The issue has been introduced by commit b69880f9ccf7e which only is in 3.14-rc,
so we don't need a stable fix for this thing.

So again, I'm replacing the Maurizio's patch with your [1/5].

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Dirk Brandewie]

Hi Rafael,

On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
>
> Based on v3.14-rc2
>
> Patch 1 removes energy reporting the patch from Maurizio Lambardi
>   intel_pstate: fix race condition in intel_pstate_init() can be dropped.
>

Any reason why patches 2-5 did not make rc3 other than timing?

Patches 2/3 can easily wait for v3.15.x

Patches 4/5 fix bugs that are in the wild.


> Patch 2-3 cleanups
>
> Patch 4-5 Baytrail bug fixes
>
> Dirk Brandewie (5):
>    intel_pstate: Remove energy reporting from pstate_sample tracepoint
>    intel_pstate: remove unneeded sample buffers
>    intel_pstate: fix pid_reset to use fixed point values
>    intel_pstate: Use LFM bus ratio as min ratio/P state
>    intel_pstate: Add support for Baytrail turbo P states
>

--Dirk

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
> Hi Rafael,

Hi,

> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com wrote:
> > From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> >
> > Based on v3.14-rc2
> >
> > Patch 1 removes energy reporting the patch from Maurizio Lambardi
> >   intel_pstate: fix race condition in intel_pstate_init() can be dropped.
> >
> 
> Any reason why patches 2-5 did not make rc3 other than timing?
> 
> Patches 2/3 can easily wait for v3.15.x
> 
> Patches 4/5 fix bugs that are in the wild.

I asked you about them, but you didn't reply:

http://marc.info/?l=linux-pm&m=139225158531023&w=4

Again, do patches [4-5/5] depend on [2-3/5]?

If not, I can queue them up for -rc4.

Thanks,
Rafael

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Dirk Brandewie]

On 02/18/2014 02:27 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
>> Hi Rafael,
>
> Hi,
>
>> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com wrote:
>>> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
>>>
>>> Based on v3.14-rc2
>>>
>>> Patch 1 removes energy reporting the patch from Maurizio Lambardi
>>>    intel_pstate: fix race condition in intel_pstate_init() can be dropped.
>>>
>>
>> Any reason why patches 2-5 did not make rc3 other than timing?
>>
>> Patches 2/3 can easily wait for v3.15.x
>>
>> Patches 4/5 fix bugs that are in the wild.
>
> I asked you about them, but you didn't reply:
>
> http://marc.info/?l=linux-pm&m=139225158531023&w=4
>
> Again, do patches [4-5/5] depend on [2-3/5]?
>
> If not, I can queue them up for -rc4.

All the patches are independent of one another.

Patch 2 is straight cleanup no functional change but reduces the memory
footprint slightly.

Patch 3 is a bug that will only be seen when the PID is reset at init time
or when a change is made to PID params via debugfs.  The problem will only
exist for one sample time since it is setting last_err in the PID.

Patch 4-5 are bugs found during Baytrail-T testing

--Dirk
>
> Thanks,
> Rafael
>

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Tuesday, February 18, 2014 03:53:48 PM Dirk Brandewie wrote:
> On 02/18/2014 02:27 PM, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
> >> Hi Rafael,
> >
> > Hi,
> >
> >> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com wrote:
> >>> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> >>>
> >>> Based on v3.14-rc2
> >>>
> >>> Patch 1 removes energy reporting the patch from Maurizio Lambardi
> >>>    intel_pstate: fix race condition in intel_pstate_init() can be dropped.
> >>>
> >>
> >> Any reason why patches 2-5 did not make rc3 other than timing?
> >>
> >> Patches 2/3 can easily wait for v3.15.x
> >>
> >> Patches 4/5 fix bugs that are in the wild.
> >
> > I asked you about them, but you didn't reply:
> >
> > http://marc.info/?l=linux-pm&m=139225158531023&w=4
> >
> > Again, do patches [4-5/5] depend on [2-3/5]?
> >
> > If not, I can queue them up for -rc4.
> 
> All the patches are independent of one another.
> 
> Patch 2 is straight cleanup no functional change but reduces the memory
> footprint slightly.
> 
> Patch 3 is a bug that will only be seen when the PID is reset at init time
> or when a change is made to PID params via debugfs.  The problem will only
> exist for one sample time since it is setting last_err in the PID.
> 
> Patch 4-5 are bugs found during Baytrail-T testing

Are there any pointers to bug reports that may be included in the changelogs
of these?

Rafael

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Dirk Brandewie]

On 02/18/2014 04:43 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 04:24:02 PM Dirk Brandewie wrote:
>> On Tuesday, February 18, 2014, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>
>>> On Tuesday, February 18, 2014 03:53:48 PM Dirk Brandewie wrote:
>>>> On 02/18/2014 02:27 PM, Rafael J. Wysocki wrote:
>>>>> On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
>>>>>> Hi Rafael,
>>>>>
>>>>> Hi,
>>>>>
>>>>>> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com <javascript:;>wrote:
>>>>>>> From: Dirk Brandewie <dirk.j.brandewie@intel.com <javascript:;>>
>>>>>>>
>>>>>>> Based on v3.14-rc2
>>>>>>>
>>>>>>> Patch 1 removes energy reporting the patch from Maurizio Lambardi
>>>>>>>     intel_pstate: fix race condition in intel_pstate_init() can be
>>> dropped.
>>>>>>>
>>>>>>
>>>>>> Any reason why patches 2-5 did not make rc3 other than timing?
>>>>>>
>>>>>> Patches 2/3 can easily wait for v3.15.x
>>>>>>
>>>>>> Patches 4/5 fix bugs that are in the wild.
>>>>>
>>>>> I asked you about them, but you didn't reply:
>>>>>
>>>>> http://marc.info/?l=linux-pm&m=139225158531023&w=4
>>>>>
>>>>> Again, do patches [4-5/5] depend on [2-3/5]?
>>>>>
>>>>> If not, I can queue them up for -rc4.
>>>>
>>>> All the patches are independent of one another.
>>>>
>>>> Patch 2 is straight cleanup no functional change but reduces the memory
>>>> footprint slightly.
>>>>
>>>> Patch 3 is a bug that will only be seen when the PID is reset at init
>>> time
>>>> or when a change is made to PID params via debugfs.  The problem will
>>> only
>>>> exist for one sample time since it is setting last_err in the PID.
>>>>
>>>> Patch 4-5 are bugs found during Baytrail-T testing
>>>
>>> Are there any pointers to bug reports that may be included in the
>>> changelogs
>>> of these?
>>
>>
>> No.  I got the reports via email.  I could probably get the reporters to
>> file bugzillas.
>
> It would be good to add information about what machines are affected
> and what the user-visible problems are to the changelogs for future
> reference.
>
> And do we want these two patches in -stable?  If so, what -stable series should
> they go into?

Patch 2 v3.10+

Patch 3 v3.12+

Patch 4/5 v3.13+

--Dirk
>
> Rafael
>

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Tuesday, February 18, 2014 04:24:02 PM Dirk Brandewie wrote:
> On Tuesday, February 18, 2014, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> 
> > On Tuesday, February 18, 2014 03:53:48 PM Dirk Brandewie wrote:
> > > On 02/18/2014 02:27 PM, Rafael J. Wysocki wrote:
> > > > On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
> > > >> Hi Rafael,
> > > >
> > > > Hi,
> > > >
> > > >> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com <javascript:;>wrote:
> > > >>> From: Dirk Brandewie <dirk.j.brandewie@intel.com <javascript:;>>
> > > >>>
> > > >>> Based on v3.14-rc2
> > > >>>
> > > >>> Patch 1 removes energy reporting the patch from Maurizio Lambardi
> > > >>>    intel_pstate: fix race condition in intel_pstate_init() can be
> > dropped.
> > > >>>
> > > >>
> > > >> Any reason why patches 2-5 did not make rc3 other than timing?
> > > >>
> > > >> Patches 2/3 can easily wait for v3.15.x
> > > >>
> > > >> Patches 4/5 fix bugs that are in the wild.
> > > >
> > > > I asked you about them, but you didn't reply:
> > > >
> > > > http://marc.info/?l=linux-pm&m=139225158531023&w=4
> > > >
> > > > Again, do patches [4-5/5] depend on [2-3/5]?
> > > >
> > > > If not, I can queue them up for -rc4.
> > >
> > > All the patches are independent of one another.
> > >
> > > Patch 2 is straight cleanup no functional change but reduces the memory
> > > footprint slightly.
> > >
> > > Patch 3 is a bug that will only be seen when the PID is reset at init
> > time
> > > or when a change is made to PID params via debugfs.  The problem will
> > only
> > > exist for one sample time since it is setting last_err in the PID.
> > >
> > > Patch 4-5 are bugs found during Baytrail-T testing
> >
> > Are there any pointers to bug reports that may be included in the
> > changelogs
> > of these?
> 
> 
> No.  I got the reports via email.  I could probably get the reporters to
> file bugzillas.

It would be good to add information about what machines are affected
and what the user-visible problems are to the changelogs for future
reference.

And do we want these two patches in -stable?  If so, what -stable series should
they go into?

Rafael

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Tuesday, February 18, 2014 04:35:26 PM Dirk Brandewie wrote:
> On 02/18/2014 04:43 PM, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 04:24:02 PM Dirk Brandewie wrote:
> >> On Tuesday, February 18, 2014, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >>
> >>> On Tuesday, February 18, 2014 03:53:48 PM Dirk Brandewie wrote:
> >>>> On 02/18/2014 02:27 PM, Rafael J. Wysocki wrote:
> >>>>> On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
> >>>>>> Hi Rafael,
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>>> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com <javascript:;>wrote:
> >>>>>>> From: Dirk Brandewie <dirk.j.brandewie@intel.com <javascript:;>>
> >>>>>>>
> >>>>>>> Based on v3.14-rc2
> >>>>>>>
> >>>>>>> Patch 1 removes energy reporting the patch from Maurizio Lambardi
> >>>>>>>     intel_pstate: fix race condition in intel_pstate_init() can be
> >>> dropped.
> >>>>>>>
> >>>>>>
> >>>>>> Any reason why patches 2-5 did not make rc3 other than timing?
> >>>>>>
> >>>>>> Patches 2/3 can easily wait for v3.15.x
> >>>>>>
> >>>>>> Patches 4/5 fix bugs that are in the wild.
> >>>>>
> >>>>> I asked you about them, but you didn't reply:
> >>>>>
> >>>>> http://marc.info/?l=linux-pm&m=139225158531023&w=4
> >>>>>
> >>>>> Again, do patches [4-5/5] depend on [2-3/5]?
> >>>>>
> >>>>> If not, I can queue them up for -rc4.
> >>>>
> >>>> All the patches are independent of one another.
> >>>>
> >>>> Patch 2 is straight cleanup no functional change but reduces the memory
> >>>> footprint slightly.
> >>>>
> >>>> Patch 3 is a bug that will only be seen when the PID is reset at init
> >>> time
> >>>> or when a change is made to PID params via debugfs.  The problem will
> >>> only
> >>>> exist for one sample time since it is setting last_err in the PID.
> >>>>
> >>>> Patch 4-5 are bugs found during Baytrail-T testing
> >>>
> >>> Are there any pointers to bug reports that may be included in the
> >>> changelogs
> >>> of these?
> >>
> >>
> >> No.  I got the reports via email.  I could probably get the reporters to
> >> file bugzillas.
> >
> > It would be good to add information about what machines are affected
> > and what the user-visible problems are to the changelogs for future
> > reference.
> >
> > And do we want these two patches in -stable?  If so, what -stable series should
> > they go into?
> 
> Patch 2 v3.10+
> 
> Patch 3 v3.12+

You said [2-3/5] were cleanups, so why do you think they are -stable material?

> Patch 4/5 v3.13+

OK

What about the bug information?  Can you please point me to the e-mail threads
where the bugs were discussed at least?

Rafael

Re: [PATCH 4/5] intel_pstate: Use LFM bus ratio as min ratio/P state

[Rafael J. Wysocki]

On Wednesday, February 12, 2014 10:01:06 AM dirk.brandewie@gmail.com wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> 
> Using LFM ratio provides better perf/watt vs min ratio.
> 
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>

This changelog is particularly insufficient.

Can you please explain what's wrong with the previous calculation and how
the change makes it better?

> ---
>  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 b80c03d..ff45ce1 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -356,7 +356,7 @@ static int byt_get_min_pstate(void)
>  {
>  	u64 value;
>  	rdmsrl(BYT_RATIOS, value);
> -	return value & 0xFF;
> +	return (value >> 8) & 0xFF;
>  }
>  
>  static int byt_get_max_pstate(void)
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Dirk Brandewie]

On 02/18/2014 04:53 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 04:35:26 PM Dirk Brandewie wrote:
>> On 02/18/2014 04:43 PM, Rafael J. Wysocki wrote:
>>> On Tuesday, February 18, 2014 04:24:02 PM Dirk Brandewie wrote:
>>>> On Tuesday, February 18, 2014, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>>>
>>>>> On Tuesday, February 18, 2014 03:53:48 PM Dirk Brandewie wrote:
>>>>>> On 02/18/2014 02:27 PM, Rafael J. Wysocki wrote:
>>>>>>> On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
>>>>>>>> Hi Rafael,
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>>> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com <javascript:;>wrote:
>>>>>>>>> From: Dirk Brandewie <dirk.j.brandewie@intel.com <javascript:;>>
>>>>>>>>>
>>>>>>>>> Based on v3.14-rc2
>>>>>>>>>
>>>>>>>>> Patch 1 removes energy reporting the patch from Maurizio Lambardi
>>>>>>>>>      intel_pstate: fix race condition in intel_pstate_init() can be
>>>>> dropped.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Any reason why patches 2-5 did not make rc3 other than timing?
>>>>>>>>
>>>>>>>> Patches 2/3 can easily wait for v3.15.x
>>>>>>>>
>>>>>>>> Patches 4/5 fix bugs that are in the wild.
>>>>>>>
>>>>>>> I asked you about them, but you didn't reply:
>>>>>>>
>>>>>>> http://marc.info/?l=linux-pm&m=139225158531023&w=4
>>>>>>>
>>>>>>> Again, do patches [4-5/5] depend on [2-3/5]?
>>>>>>>
>>>>>>> If not, I can queue them up for -rc4.
>>>>>>
>>>>>> All the patches are independent of one another.
>>>>>>
>>>>>> Patch 2 is straight cleanup no functional change but reduces the memory
>>>>>> footprint slightly.
>>>>>>
>>>>>> Patch 3 is a bug that will only be seen when the PID is reset at init
>>>>> time
>>>>>> or when a change is made to PID params via debugfs.  The problem will
>>>>> only
>>>>>> exist for one sample time since it is setting last_err in the PID.
>>>>>>
>>>>>> Patch 4-5 are bugs found during Baytrail-T testing
>>>>>
>>>>> Are there any pointers to bug reports that may be included in the
>>>>> changelogs
>>>>> of these?
>>>>
>>>>
>>>> No.  I got the reports via email.  I could probably get the reporters to
>>>> file bugzillas.
>>>
>>> It would be good to add information about what machines are affected
>>> and what the user-visible problems are to the changelogs for future
>>> reference.
>>>
>>> And do we want these two patches in -stable?  If so, what -stable series should
>>> they go into?
>>
>> Patch 2 v3.10+
>>
>> Patch 3 v3.12+
>
> You said [2-3/5] were cleanups, so why do you think they are -stable material?

I misspoke these are not stable material

>
>> Patch 4/5 v3.13+
>
> OK
>
> What about the bug information?  Can you please point me to the e-mail threads
> where the bugs were discussed at least?
>
> Rafael
>

[PATCH v2] intel_pstate: Use LFM bus ratio as min ratio/P state

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

LFM (max efficiency ratio) is the max frequency at minimum voltage
supported by the processor.  Using LFM as the minimum P state
increases performmance without affecting power. By not using P states
below LFM we avoid using P states that are less power efficient.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@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 b80c03d..ff45ce1 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -356,7 +356,7 @@ static int byt_get_min_pstate(void)
 {
 	u64 value;
 	rdmsrl(BYT_RATIOS, value);
-	return value & 0xFF;
+	return (value >> 8) & 0xFF;
 }
 
 static int byt_get_max_pstate(void)
-- 
1.8.3.1

Re: [PATCH 0/5] intel_pstate updates for v3.14-rcX

[Rafael J. Wysocki]

On Wednesday, February 19, 2014 10:26:52 AM Dirk Brandewie wrote:
> On 02/18/2014 04:53 PM, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 04:35:26 PM Dirk Brandewie wrote:
> >> On 02/18/2014 04:43 PM, Rafael J. Wysocki wrote:
> >>> On Tuesday, February 18, 2014 04:24:02 PM Dirk Brandewie wrote:
> >>>> On Tuesday, February 18, 2014, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >>>>
> >>>>> On Tuesday, February 18, 2014 03:53:48 PM Dirk Brandewie wrote:
> >>>>>> On 02/18/2014 02:27 PM, Rafael J. Wysocki wrote:
> >>>>>>> On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
> >>>>>>>> Hi Rafael,
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>>> On 02/12/2014 10:01 AM, dirk.brandewie@gmail.com <javascript:;>wrote:
> >>>>>>>>> From: Dirk Brandewie <dirk.j.brandewie@intel.com <javascript:;>>
> >>>>>>>>>
> >>>>>>>>> Based on v3.14-rc2
> >>>>>>>>>
> >>>>>>>>> Patch 1 removes energy reporting the patch from Maurizio Lambardi
> >>>>>>>>>      intel_pstate: fix race condition in intel_pstate_init() can be
> >>>>> dropped.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Any reason why patches 2-5 did not make rc3 other than timing?
> >>>>>>>>
> >>>>>>>> Patches 2/3 can easily wait for v3.15.x
> >>>>>>>>
> >>>>>>>> Patches 4/5 fix bugs that are in the wild.
> >>>>>>>
> >>>>>>> I asked you about them, but you didn't reply:
> >>>>>>>
> >>>>>>> http://marc.info/?l=linux-pm&m=139225158531023&w=4
> >>>>>>>
> >>>>>>> Again, do patches [4-5/5] depend on [2-3/5]?
> >>>>>>>
> >>>>>>> If not, I can queue them up for -rc4.
> >>>>>>
> >>>>>> All the patches are independent of one another.
> >>>>>>
> >>>>>> Patch 2 is straight cleanup no functional change but reduces the memory
> >>>>>> footprint slightly.
> >>>>>>
> >>>>>> Patch 3 is a bug that will only be seen when the PID is reset at init
> >>>>> time
> >>>>>> or when a change is made to PID params via debugfs.  The problem will
> >>>>> only
> >>>>>> exist for one sample time since it is setting last_err in the PID.
> >>>>>>
> >>>>>> Patch 4-5 are bugs found during Baytrail-T testing
> >>>>>
> >>>>> Are there any pointers to bug reports that may be included in the
> >>>>> changelogs
> >>>>> of these?
> >>>>
> >>>>
> >>>> No.  I got the reports via email.  I could probably get the reporters to
> >>>> file bugzillas.
> >>>
> >>> It would be good to add information about what machines are affected
> >>> and what the user-visible problems are to the changelogs for future
> >>> reference.
> >>>
> >>> And do we want these two patches in -stable?  If so, what -stable series should
> >>> they go into?
> >>
> >> Patch 2 v3.10+
> >>
> >> Patch 3 v3.12+
> >
> > You said [2-3/5] were cleanups, so why do you think they are -stable material?
> 
> I misspoke these are not stable material

OK

Re: [PATCH v2] intel_pstate: Use LFM bus ratio as min ratio/P state

[Rafael J. Wysocki]

On Wednesday, February 19, 2014 10:35:25 AM dirk.brandewie@gmail.com wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> 
> LFM (max efficiency ratio) is the max frequency at minimum voltage
> supported by the processor.  Using LFM as the minimum P state
> increases performmance without affecting power. By not using P states
> below LFM we avoid using P states that are less power efficient.
> 
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>

Thanks!  I've replaced the original changelog with the one above.

> ---
>  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 b80c03d..ff45ce1 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -356,7 +356,7 @@ static int byt_get_min_pstate(void)
>  {
>  	u64 value;
>  	rdmsrl(BYT_RATIOS, value);
> -	return value & 0xFF;
> +	return (value >> 8) & 0xFF;
>  }
>  
>  static int byt_get_max_pstate(void)
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.