From: Dirk Brandewie <[email protected]>
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
From: Dirk Brandewie <[email protected]>
Using LFM ratio provides better perf/watt vs min ratio.
Signed-off-by: Dirk Brandewie <[email protected]>
---
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
From: Dirk Brandewie <[email protected]>
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 <[email protected]>
---
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
From: Dirk Brandewie <[email protected]>
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 <[email protected]>
---
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
From: Dirk Brandewie <[email protected]>
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 <[email protected]>
---
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
From: Dirk Brandewie <[email protected]>
A documentation update exposed the existance of the turbo ratio
register. Update baytrail support to use the turbo range.
Signed-off-by: Dirk Brandewie <[email protected]>
---
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
On Wednesday, February 12, 2014 10:01:02 AM [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> 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.
On 2014/2/13 2:01, [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> 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 <[email protected]>
> ---
> 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;
> }
>
>
On Thursday, February 13, 2014 01:47:43 AM Rafael J. Wysocki wrote:
> On Wednesday, February 12, 2014 10:01:02 AM [email protected] wrote:
> > From: Dirk Brandewie <[email protected]>
> >
> > 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.
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 [email protected] wrote:
> > > From: Dirk Brandewie <[email protected]>
> > >
> > > 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.
Hi Rafael,
On 02/12/2014 10:01 AM, [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> 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
On Tuesday, February 18, 2014 12:29:54 PM Dirk Brandewie wrote:
> Hi Rafael,
Hi,
> On 02/12/2014 10:01 AM, [email protected] wrote:
> > From: Dirk Brandewie <[email protected]>
> >
> > 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
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, [email protected] wrote:
>>> From: Dirk Brandewie <[email protected]>
>>>
>>> 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
>
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, [email protected] wrote:
> >>> From: Dirk Brandewie <[email protected]>
> >>>
> >>> 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
On Tuesday, February 18, 2014 04:24:02 PM Dirk Brandewie wrote:
> On Tuesday, February 18, 2014, Rafael J. Wysocki <[email protected]> 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, [email protected] <javascript:;>wrote:
> > > >>> From: Dirk Brandewie <[email protected] <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
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 <[email protected]> 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, [email protected] <javascript:;>wrote:
>>>>>>> From: Dirk Brandewie <[email protected] <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
>
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 <[email protected]> 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, [email protected] <javascript:;>wrote:
> >>>>>>> From: Dirk Brandewie <[email protected] <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
On Wednesday, February 12, 2014 10:01:06 AM [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> Using LFM ratio provides better perf/watt vs min ratio.
>
> Signed-off-by: Dirk Brandewie <[email protected]>
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.
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 <[email protected]> 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, [email protected] <javascript:;>wrote:
>>>>>>>>> From: Dirk Brandewie <[email protected] <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
>
From: Dirk Brandewie <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]> 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, [email protected] <javascript:;>wrote:
> >>>>>>>>> From: Dirk Brandewie <[email protected] <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
On Wednesday, February 19, 2014 10:35:25 AM [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> 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 <[email protected]>
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.