Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
On 05/07/2015 04:05 PM, Shilpasri G Bhat wrote:
>
>
> On 05/05/2015 02:08 PM, Preeti U Murthy wrote:
>> On 05/05/2015 11:36 AM, Shilpasri G Bhat wrote:
>>> Hi Preeti,
>>>
>>> On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
Hi Shilpa,
On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
> The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
> max allowed frequency for that chip if the chip exceeds its power or
> temperature limits. As Pmax capping is a chip level condition report
> this throttling behavior at chip level and also do not set the global
> 'throttled' on Pmax capping instead set the per-chip throttled
> variable. Report unthrottling if Pmax is restored after throttling.
>
> This patch adds a structure to store chip id and throttled state of
> the chip.
>
> Signed-off-by: Shilpasri G Bhat
> ---
> drivers/cpufreq/powernv-cpufreq.c | 59
> ---
> 1 file changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c
> b/drivers/cpufreq/powernv-cpufreq.c
> index ebef0d8..d0c18c9 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -42,6 +43,13 @@
> static struct cpufreq_frequency_table
> powernv_freqs[POWERNV_MAX_PSTATES+1];
> static bool rebooting, throttled;
>
> +static struct chip {
> + unsigned int id;
> + bool throttled;
> +} *chips;
> +
> +static int nr_chips;
> +
> /*
> * Note: The set of pstates consists of contiguous integers, the
> * smallest of which is indicated by powernv_pstate_info.min, the
> @@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
> static void powernv_cpufreq_throttle_check(unsigned int cpu)
> {
> unsigned long pmsr;
> - int pmsr_pmax, pmsr_lp;
> + int pmsr_pmax, pmsr_lp, i;
>
> pmsr = get_pmspr(SPRN_PMSR);
>
> + for (i = 0; i < nr_chips; i++)
> + if (chips[i].id == cpu_to_chip_id(cpu))
> + break;
> +
> /* Check for Pmax Capping */
> pmsr_pmax = (s8)PMSR_MAX(pmsr);
> if (pmsr_pmax != powernv_pstate_info.max) {
> - throttled = true;
> - pr_info("CPU %d Pmax is reduced to %d\n", cpu, pmsr_pmax);
> - pr_info("Max allowed Pstate is capped\n");
> + if (chips[i].throttled)
> + goto next;
> + chips[i].throttled = true;
> + pr_info("CPU %d on Chip %u has Pmax reduced to %d\n", cpu,
> + chips[i].id, pmsr_pmax);
> + } else if (chips[i].throttled) {
> + chips[i].throttled = false;
Is this check on pmax sufficient to indicate that the chip is unthrottled ?
>>>
>>> Unthrottling due to Pmax uncapping here is specific to a chip. So it is
>>> sufficient to decide throttling/unthrottling when OCC is active for that
>>> chip.
>>
>> Ok then we can perhaps exit after detecting unthrottling here.
>
> This won't work for older firmwares which do not clear "Frequency control
> enabled bit" on OCC reset cycle. So let us check for remaining two conditions
> on
> unthrottling as well.
ok.
>
>>>
> + pr_info("CPU %d on Chip %u has Pmax restored to %d\n", cpu,
> + chips[i].id, pmsr_pmax);
> }
>
> /*
>* Check for Psafe by reading LocalPstate
>* or check if Psafe_mode_active is set in PMSR.
>*/
> +next:
> pmsr_lp = (s8)PMSR_LP(pmsr);
> if ((pmsr_lp < powernv_pstate_info.min) ||
> (pmsr & PMSR_PSAFE_ENABLE)) {
> @@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver
> = {
> .attr = powernv_cpu_freq_attr,
What about the situation where although occ is active, this particular
chip has been throttled and we end up repeatedly reporting "pstate set
to safe" and "frequency control disabled from OS" ? Should we not have a
check on (chips[i].throttled) before reporting an anomaly for these two
scenarios as well just like you have for pmsr_pmax ?
>>>
>>> We will not have "Psafe" and "frequency control disabled" repeatedly printed
>>> because of global variable 'throttled', which is set to true on passing any
>>> of
>>> these two conditions.
>>>
>>> It is quite unlikely behavior to have only one chip in "Psafe" or "frequency
>>> control disabled" state. These two conditions are most likely to happen
>>> during
>>> an OCC reset cycle which will occur across all chips.
>>
>> Let us then add a comment to indicate that Psafe and frequency control
>> disabled conditions will fail *only if OCC is inactive* and not
>> otherwise and that this is a
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
On 05/05/2015 02:08 PM, Preeti U Murthy wrote:
> On 05/05/2015 11:36 AM, Shilpasri G Bhat wrote:
>> Hi Preeti,
>>
>> On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
>>> Hi Shilpa,
>>>
>>> On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
max allowed frequency for that chip if the chip exceeds its power or
temperature limits. As Pmax capping is a chip level condition report
this throttling behavior at chip level and also do not set the global
'throttled' on Pmax capping instead set the per-chip throttled
variable. Report unthrottling if Pmax is restored after throttling.
This patch adds a structure to store chip id and throttled state of
the chip.
Signed-off-by: Shilpasri G Bhat
---
drivers/cpufreq/powernv-cpufreq.c | 59
---
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c
b/drivers/cpufreq/powernv-cpufreq.c
index ebef0d8..d0c18c9 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -42,6 +43,13 @@
static struct cpufreq_frequency_table
powernv_freqs[POWERNV_MAX_PSTATES+1];
static bool rebooting, throttled;
+static struct chip {
+ unsigned int id;
+ bool throttled;
+} *chips;
+
+static int nr_chips;
+
/*
* Note: The set of pstates consists of contiguous integers, the
* smallest of which is indicated by powernv_pstate_info.min, the
@@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
static void powernv_cpufreq_throttle_check(unsigned int cpu)
{
unsigned long pmsr;
- int pmsr_pmax, pmsr_lp;
+ int pmsr_pmax, pmsr_lp, i;
pmsr = get_pmspr(SPRN_PMSR);
+ for (i = 0; i < nr_chips; i++)
+ if (chips[i].id == cpu_to_chip_id(cpu))
+ break;
+
/* Check for Pmax Capping */
pmsr_pmax = (s8)PMSR_MAX(pmsr);
if (pmsr_pmax != powernv_pstate_info.max) {
- throttled = true;
- pr_info("CPU %d Pmax is reduced to %d\n", cpu, pmsr_pmax);
- pr_info("Max allowed Pstate is capped\n");
+ if (chips[i].throttled)
+ goto next;
+ chips[i].throttled = true;
+ pr_info("CPU %d on Chip %u has Pmax reduced to %d\n", cpu,
+ chips[i].id, pmsr_pmax);
+ } else if (chips[i].throttled) {
+ chips[i].throttled = false;
>>>
>>> Is this check on pmax sufficient to indicate that the chip is unthrottled ?
>>
>> Unthrottling due to Pmax uncapping here is specific to a chip. So it is
>> sufficient to decide throttling/unthrottling when OCC is active for that
>> chip.
>
> Ok then we can perhaps exit after detecting unthrottling here.
This won't work for older firmwares which do not clear "Frequency control
enabled bit" on OCC reset cycle. So let us check for remaining two conditions on
unthrottling as well.
>>
>>>
+ pr_info("CPU %d on Chip %u has Pmax restored to %d\n", cpu,
+ chips[i].id, pmsr_pmax);
}
/*
* Check for Psafe by reading LocalPstate
* or check if Psafe_mode_active is set in PMSR.
*/
+next:
pmsr_lp = (s8)PMSR_LP(pmsr);
if ((pmsr_lp < powernv_pstate_info.min) ||
(pmsr & PMSR_PSAFE_ENABLE)) {
@@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver =
{
.attr = powernv_cpu_freq_attr,
>>>
>>> What about the situation where although occ is active, this particular
>>> chip has been throttled and we end up repeatedly reporting "pstate set
>>> to safe" and "frequency control disabled from OS" ? Should we not have a
>>> check on (chips[i].throttled) before reporting an anomaly for these two
>>> scenarios as well just like you have for pmsr_pmax ?
>>
>> We will not have "Psafe" and "frequency control disabled" repeatedly printed
>> because of global variable 'throttled', which is set to true on passing any
>> of
>> these two conditions.
>>
>> It is quite unlikely behavior to have only one chip in "Psafe" or "frequency
>> control disabled" state. These two conditions are most likely to happen
>> during
>> an OCC reset cycle which will occur across all chips.
>
> Let us then add a comment to indicate that Psafe and frequency control
> disabled conditions will fail *only if OCC is inactive* and not
> otherwise and that this is a system wide phenomenon.
>
I agree that adding a comment here will clear global vs local throttling
scenarios, but this will contradict the architectural
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
On 05/05/2015 02:08 PM, Preeti U Murthy wrote:
On 05/05/2015 11:36 AM, Shilpasri G Bhat wrote:
Hi Preeti,
On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
Hi Shilpa,
On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
max allowed frequency for that chip if the chip exceeds its power or
temperature limits. As Pmax capping is a chip level condition report
this throttling behavior at chip level and also do not set the global
'throttled' on Pmax capping instead set the per-chip throttled
variable. Report unthrottling if Pmax is restored after throttling.
This patch adds a structure to store chip id and throttled state of
the chip.
Signed-off-by: Shilpasri G Bhat shilpa.b...@linux.vnet.ibm.com
---
drivers/cpufreq/powernv-cpufreq.c | 59
---
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c
b/drivers/cpufreq/powernv-cpufreq.c
index ebef0d8..d0c18c9 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -27,6 +27,7 @@
#include linux/smp.h
#include linux/of.h
#include linux/reboot.h
+#include linux/slab.h
#include asm/cputhreads.h
#include asm/firmware.h
@@ -42,6 +43,13 @@
static struct cpufreq_frequency_table
powernv_freqs[POWERNV_MAX_PSTATES+1];
static bool rebooting, throttled;
+static struct chip {
+ unsigned int id;
+ bool throttled;
+} *chips;
+
+static int nr_chips;
+
/*
* Note: The set of pstates consists of contiguous integers, the
* smallest of which is indicated by powernv_pstate_info.min, the
@@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
static void powernv_cpufreq_throttle_check(unsigned int cpu)
{
unsigned long pmsr;
- int pmsr_pmax, pmsr_lp;
+ int pmsr_pmax, pmsr_lp, i;
pmsr = get_pmspr(SPRN_PMSR);
+ for (i = 0; i nr_chips; i++)
+ if (chips[i].id == cpu_to_chip_id(cpu))
+ break;
+
/* Check for Pmax Capping */
pmsr_pmax = (s8)PMSR_MAX(pmsr);
if (pmsr_pmax != powernv_pstate_info.max) {
- throttled = true;
- pr_info(CPU %d Pmax is reduced to %d\n, cpu, pmsr_pmax);
- pr_info(Max allowed Pstate is capped\n);
+ if (chips[i].throttled)
+ goto next;
+ chips[i].throttled = true;
+ pr_info(CPU %d on Chip %u has Pmax reduced to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
+ } else if (chips[i].throttled) {
+ chips[i].throttled = false;
Is this check on pmax sufficient to indicate that the chip is unthrottled ?
Unthrottling due to Pmax uncapping here is specific to a chip. So it is
sufficient to decide throttling/unthrottling when OCC is active for that
chip.
Ok then we can perhaps exit after detecting unthrottling here.
This won't work for older firmwares which do not clear Frequency control
enabled bit on OCC reset cycle. So let us check for remaining two conditions on
unthrottling as well.
+ pr_info(CPU %d on Chip %u has Pmax restored to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
}
/*
* Check for Psafe by reading LocalPstate
* or check if Psafe_mode_active is set in PMSR.
*/
+next:
pmsr_lp = (s8)PMSR_LP(pmsr);
if ((pmsr_lp powernv_pstate_info.min) ||
(pmsr PMSR_PSAFE_ENABLE)) {
@@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver =
{
.attr = powernv_cpu_freq_attr,
What about the situation where although occ is active, this particular
chip has been throttled and we end up repeatedly reporting pstate set
to safe and frequency control disabled from OS ? Should we not have a
check on (chips[i].throttled) before reporting an anomaly for these two
scenarios as well just like you have for pmsr_pmax ?
We will not have Psafe and frequency control disabled repeatedly printed
because of global variable 'throttled', which is set to true on passing any
of
these two conditions.
It is quite unlikely behavior to have only one chip in Psafe or frequency
control disabled state. These two conditions are most likely to happen
during
an OCC reset cycle which will occur across all chips.
Let us then add a comment to indicate that Psafe and frequency control
disabled conditions will fail *only if OCC is inactive* and not
otherwise and that this is a system wide phenomenon.
I agree that adding a comment here will clear global vs local throttling
scenarios, but this will contradict the architectural design of OCC wherein it
can independently go to Psafe and frequency control disabled state. It is
the implementation in FSP today that has made the above two states global. My
point is adding a comment here may be confusing if at all for the future
firmwares this implementation is changed. Having said that the current patch set
still seems
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
On 05/07/2015 04:05 PM, Shilpasri G Bhat wrote:
On 05/05/2015 02:08 PM, Preeti U Murthy wrote:
On 05/05/2015 11:36 AM, Shilpasri G Bhat wrote:
Hi Preeti,
On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
Hi Shilpa,
On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
max allowed frequency for that chip if the chip exceeds its power or
temperature limits. As Pmax capping is a chip level condition report
this throttling behavior at chip level and also do not set the global
'throttled' on Pmax capping instead set the per-chip throttled
variable. Report unthrottling if Pmax is restored after throttling.
This patch adds a structure to store chip id and throttled state of
the chip.
Signed-off-by: Shilpasri G Bhat shilpa.b...@linux.vnet.ibm.com
---
drivers/cpufreq/powernv-cpufreq.c | 59
---
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c
b/drivers/cpufreq/powernv-cpufreq.c
index ebef0d8..d0c18c9 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -27,6 +27,7 @@
#include linux/smp.h
#include linux/of.h
#include linux/reboot.h
+#include linux/slab.h
#include asm/cputhreads.h
#include asm/firmware.h
@@ -42,6 +43,13 @@
static struct cpufreq_frequency_table
powernv_freqs[POWERNV_MAX_PSTATES+1];
static bool rebooting, throttled;
+static struct chip {
+ unsigned int id;
+ bool throttled;
+} *chips;
+
+static int nr_chips;
+
/*
* Note: The set of pstates consists of contiguous integers, the
* smallest of which is indicated by powernv_pstate_info.min, the
@@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
static void powernv_cpufreq_throttle_check(unsigned int cpu)
{
unsigned long pmsr;
- int pmsr_pmax, pmsr_lp;
+ int pmsr_pmax, pmsr_lp, i;
pmsr = get_pmspr(SPRN_PMSR);
+ for (i = 0; i nr_chips; i++)
+ if (chips[i].id == cpu_to_chip_id(cpu))
+ break;
+
/* Check for Pmax Capping */
pmsr_pmax = (s8)PMSR_MAX(pmsr);
if (pmsr_pmax != powernv_pstate_info.max) {
- throttled = true;
- pr_info(CPU %d Pmax is reduced to %d\n, cpu, pmsr_pmax);
- pr_info(Max allowed Pstate is capped\n);
+ if (chips[i].throttled)
+ goto next;
+ chips[i].throttled = true;
+ pr_info(CPU %d on Chip %u has Pmax reduced to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
+ } else if (chips[i].throttled) {
+ chips[i].throttled = false;
Is this check on pmax sufficient to indicate that the chip is unthrottled ?
Unthrottling due to Pmax uncapping here is specific to a chip. So it is
sufficient to decide throttling/unthrottling when OCC is active for that
chip.
Ok then we can perhaps exit after detecting unthrottling here.
This won't work for older firmwares which do not clear Frequency control
enabled bit on OCC reset cycle. So let us check for remaining two conditions
on
unthrottling as well.
ok.
+ pr_info(CPU %d on Chip %u has Pmax restored to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
}
/*
* Check for Psafe by reading LocalPstate
* or check if Psafe_mode_active is set in PMSR.
*/
+next:
pmsr_lp = (s8)PMSR_LP(pmsr);
if ((pmsr_lp powernv_pstate_info.min) ||
(pmsr PMSR_PSAFE_ENABLE)) {
@@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver
= {
.attr = powernv_cpu_freq_attr,
What about the situation where although occ is active, this particular
chip has been throttled and we end up repeatedly reporting pstate set
to safe and frequency control disabled from OS ? Should we not have a
check on (chips[i].throttled) before reporting an anomaly for these two
scenarios as well just like you have for pmsr_pmax ?
We will not have Psafe and frequency control disabled repeatedly printed
because of global variable 'throttled', which is set to true on passing any
of
these two conditions.
It is quite unlikely behavior to have only one chip in Psafe or frequency
control disabled state. These two conditions are most likely to happen
during
an OCC reset cycle which will occur across all chips.
Let us then add a comment to indicate that Psafe and frequency control
disabled conditions will fail *only if OCC is inactive* and not
otherwise and that this is a system wide phenomenon.
I agree that adding a comment here will clear global vs local throttling
scenarios, but this will contradict the architectural design of OCC wherein it
can independently go to Psafe and frequency control disabled state. It is
the implementation in FSP today that has made the above two states global. My
point is adding a comment here may be confusing if at all for the future
firmwares this implementation is changed. Having said that
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
On 05/05/2015 11:36 AM, Shilpasri G Bhat wrote:
> Hi Preeti,
>
> On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
>> Hi Shilpa,
>>
>> On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
>>> The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
>>> max allowed frequency for that chip if the chip exceeds its power or
>>> temperature limits. As Pmax capping is a chip level condition report
>>> this throttling behavior at chip level and also do not set the global
>>> 'throttled' on Pmax capping instead set the per-chip throttled
>>> variable. Report unthrottling if Pmax is restored after throttling.
>>>
>>> This patch adds a structure to store chip id and throttled state of
>>> the chip.
>>>
>>> Signed-off-by: Shilpasri G Bhat
>>> ---
>>> drivers/cpufreq/powernv-cpufreq.c | 59
>>> ---
>>> 1 file changed, 55 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/powernv-cpufreq.c
>>> b/drivers/cpufreq/powernv-cpufreq.c
>>> index ebef0d8..d0c18c9 100644
>>> --- a/drivers/cpufreq/powernv-cpufreq.c
>>> +++ b/drivers/cpufreq/powernv-cpufreq.c
>>> @@ -27,6 +27,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> #include
>>> #include
>>> @@ -42,6 +43,13 @@
>>> static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
>>> static bool rebooting, throttled;
>>>
>>> +static struct chip {
>>> + unsigned int id;
>>> + bool throttled;
>>> +} *chips;
>>> +
>>> +static int nr_chips;
>>> +
>>> /*
>>> * Note: The set of pstates consists of contiguous integers, the
>>> * smallest of which is indicated by powernv_pstate_info.min, the
>>> @@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
>>> static void powernv_cpufreq_throttle_check(unsigned int cpu)
>>> {
>>> unsigned long pmsr;
>>> - int pmsr_pmax, pmsr_lp;
>>> + int pmsr_pmax, pmsr_lp, i;
>>>
>>> pmsr = get_pmspr(SPRN_PMSR);
>>>
>>> + for (i = 0; i < nr_chips; i++)
>>> + if (chips[i].id == cpu_to_chip_id(cpu))
>>> + break;
>>> +
>>> /* Check for Pmax Capping */
>>> pmsr_pmax = (s8)PMSR_MAX(pmsr);
>>> if (pmsr_pmax != powernv_pstate_info.max) {
>>> - throttled = true;
>>> - pr_info("CPU %d Pmax is reduced to %d\n", cpu, pmsr_pmax);
>>> - pr_info("Max allowed Pstate is capped\n");
>>> + if (chips[i].throttled)
>>> + goto next;
>>> + chips[i].throttled = true;
>>> + pr_info("CPU %d on Chip %u has Pmax reduced to %d\n", cpu,
>>> + chips[i].id, pmsr_pmax);
>>> + } else if (chips[i].throttled) {
>>> + chips[i].throttled = false;
>>
>> Is this check on pmax sufficient to indicate that the chip is unthrottled ?
>
> Unthrottling due to Pmax uncapping here is specific to a chip. So it is
> sufficient to decide throttling/unthrottling when OCC is active for that chip.
Ok then we can perhaps exit after detecting unthrottling here.
>
>>
>>> + pr_info("CPU %d on Chip %u has Pmax restored to %d\n", cpu,
>>> + chips[i].id, pmsr_pmax);
>>> }
>>>
>>> /*
>>> * Check for Psafe by reading LocalPstate
>>> * or check if Psafe_mode_active is set in PMSR.
>>> */
>>> +next:
>>> pmsr_lp = (s8)PMSR_LP(pmsr);
>>> if ((pmsr_lp < powernv_pstate_info.min) ||
>>> (pmsr & PMSR_PSAFE_ENABLE)) {
>>> @@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
>>> .attr = powernv_cpu_freq_attr,
>>
>> What about the situation where although occ is active, this particular
>> chip has been throttled and we end up repeatedly reporting "pstate set
>> to safe" and "frequency control disabled from OS" ? Should we not have a
>> check on (chips[i].throttled) before reporting an anomaly for these two
>> scenarios as well just like you have for pmsr_pmax ?
>
> We will not have "Psafe" and "frequency control disabled" repeatedly printed
> because of global variable 'throttled', which is set to true on passing any of
> these two conditions.
>
> It is quite unlikely behavior to have only one chip in "Psafe" or "frequency
> control disabled" state. These two conditions are most likely to happen during
> an OCC reset cycle which will occur across all chips.
Let us then add a comment to indicate that Psafe and frequency control
disabled conditions will fail *only if OCC is inactive* and not
otherwise and that this is a system wide phenomenon.
Regards
Preeti U Murthy
>
> Thanks and Regards,
> Shilpa
>
> ___
> Linuxppc-dev mailing list
> linuxppc-...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
Hi Preeti,
On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
> Hi Shilpa,
>
> On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
>> The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
>> max allowed frequency for that chip if the chip exceeds its power or
>> temperature limits. As Pmax capping is a chip level condition report
>> this throttling behavior at chip level and also do not set the global
>> 'throttled' on Pmax capping instead set the per-chip throttled
>> variable. Report unthrottling if Pmax is restored after throttling.
>>
>> This patch adds a structure to store chip id and throttled state of
>> the chip.
>>
>> Signed-off-by: Shilpasri G Bhat
>> ---
>> drivers/cpufreq/powernv-cpufreq.c | 59
>> ---
>> 1 file changed, 55 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/powernv-cpufreq.c
>> b/drivers/cpufreq/powernv-cpufreq.c
>> index ebef0d8..d0c18c9 100644
>> --- a/drivers/cpufreq/powernv-cpufreq.c
>> +++ b/drivers/cpufreq/powernv-cpufreq.c
>> @@ -27,6 +27,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -42,6 +43,13 @@
>> static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
>> static bool rebooting, throttled;
>>
>> +static struct chip {
>> +unsigned int id;
>> +bool throttled;
>> +} *chips;
>> +
>> +static int nr_chips;
>> +
>> /*
>> * Note: The set of pstates consists of contiguous integers, the
>> * smallest of which is indicated by powernv_pstate_info.min, the
>> @@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
>> static void powernv_cpufreq_throttle_check(unsigned int cpu)
>> {
>> unsigned long pmsr;
>> -int pmsr_pmax, pmsr_lp;
>> +int pmsr_pmax, pmsr_lp, i;
>>
>> pmsr = get_pmspr(SPRN_PMSR);
>>
>> +for (i = 0; i < nr_chips; i++)
>> +if (chips[i].id == cpu_to_chip_id(cpu))
>> +break;
>> +
>> /* Check for Pmax Capping */
>> pmsr_pmax = (s8)PMSR_MAX(pmsr);
>> if (pmsr_pmax != powernv_pstate_info.max) {
>> -throttled = true;
>> -pr_info("CPU %d Pmax is reduced to %d\n", cpu, pmsr_pmax);
>> -pr_info("Max allowed Pstate is capped\n");
>> +if (chips[i].throttled)
>> +goto next;
>> +chips[i].throttled = true;
>> +pr_info("CPU %d on Chip %u has Pmax reduced to %d\n", cpu,
>> +chips[i].id, pmsr_pmax);
>> +} else if (chips[i].throttled) {
>> +chips[i].throttled = false;
>
> Is this check on pmax sufficient to indicate that the chip is unthrottled ?
Unthrottling due to Pmax uncapping here is specific to a chip. So it is
sufficient to decide throttling/unthrottling when OCC is active for that chip.
>
>> +pr_info("CPU %d on Chip %u has Pmax restored to %d\n", cpu,
>> +chips[i].id, pmsr_pmax);
>> }
>>
>> /*
>> * Check for Psafe by reading LocalPstate
>> * or check if Psafe_mode_active is set in PMSR.
>> */
>> +next:
>> pmsr_lp = (s8)PMSR_LP(pmsr);
>> if ((pmsr_lp < powernv_pstate_info.min) ||
>> (pmsr & PMSR_PSAFE_ENABLE)) {
>> @@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
>> .attr = powernv_cpu_freq_attr,
>
> What about the situation where although occ is active, this particular
> chip has been throttled and we end up repeatedly reporting "pstate set
> to safe" and "frequency control disabled from OS" ? Should we not have a
> check on (chips[i].throttled) before reporting an anomaly for these two
> scenarios as well just like you have for pmsr_pmax ?
We will not have "Psafe" and "frequency control disabled" repeatedly printed
because of global variable 'throttled', which is set to true on passing any of
these two conditions.
It is quite unlikely behavior to have only one chip in "Psafe" or "frequency
control disabled" state. These two conditions are most likely to happen during
an OCC reset cycle which will occur across all chips.
Thanks and Regards,
Shilpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
Hi Preeti,
On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
Hi Shilpa,
On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
max allowed frequency for that chip if the chip exceeds its power or
temperature limits. As Pmax capping is a chip level condition report
this throttling behavior at chip level and also do not set the global
'throttled' on Pmax capping instead set the per-chip throttled
variable. Report unthrottling if Pmax is restored after throttling.
This patch adds a structure to store chip id and throttled state of
the chip.
Signed-off-by: Shilpasri G Bhat shilpa.b...@linux.vnet.ibm.com
---
drivers/cpufreq/powernv-cpufreq.c | 59
---
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c
b/drivers/cpufreq/powernv-cpufreq.c
index ebef0d8..d0c18c9 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -27,6 +27,7 @@
#include linux/smp.h
#include linux/of.h
#include linux/reboot.h
+#include linux/slab.h
#include asm/cputhreads.h
#include asm/firmware.h
@@ -42,6 +43,13 @@
static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
static bool rebooting, throttled;
+static struct chip {
+unsigned int id;
+bool throttled;
+} *chips;
+
+static int nr_chips;
+
/*
* Note: The set of pstates consists of contiguous integers, the
* smallest of which is indicated by powernv_pstate_info.min, the
@@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
static void powernv_cpufreq_throttle_check(unsigned int cpu)
{
unsigned long pmsr;
-int pmsr_pmax, pmsr_lp;
+int pmsr_pmax, pmsr_lp, i;
pmsr = get_pmspr(SPRN_PMSR);
+for (i = 0; i nr_chips; i++)
+if (chips[i].id == cpu_to_chip_id(cpu))
+break;
+
/* Check for Pmax Capping */
pmsr_pmax = (s8)PMSR_MAX(pmsr);
if (pmsr_pmax != powernv_pstate_info.max) {
-throttled = true;
-pr_info(CPU %d Pmax is reduced to %d\n, cpu, pmsr_pmax);
-pr_info(Max allowed Pstate is capped\n);
+if (chips[i].throttled)
+goto next;
+chips[i].throttled = true;
+pr_info(CPU %d on Chip %u has Pmax reduced to %d\n, cpu,
+chips[i].id, pmsr_pmax);
+} else if (chips[i].throttled) {
+chips[i].throttled = false;
Is this check on pmax sufficient to indicate that the chip is unthrottled ?
Unthrottling due to Pmax uncapping here is specific to a chip. So it is
sufficient to decide throttling/unthrottling when OCC is active for that chip.
+pr_info(CPU %d on Chip %u has Pmax restored to %d\n, cpu,
+chips[i].id, pmsr_pmax);
}
/*
* Check for Psafe by reading LocalPstate
* or check if Psafe_mode_active is set in PMSR.
*/
+next:
pmsr_lp = (s8)PMSR_LP(pmsr);
if ((pmsr_lp powernv_pstate_info.min) ||
(pmsr PMSR_PSAFE_ENABLE)) {
@@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
.attr = powernv_cpu_freq_attr,
What about the situation where although occ is active, this particular
chip has been throttled and we end up repeatedly reporting pstate set
to safe and frequency control disabled from OS ? Should we not have a
check on (chips[i].throttled) before reporting an anomaly for these two
scenarios as well just like you have for pmsr_pmax ?
We will not have Psafe and frequency control disabled repeatedly printed
because of global variable 'throttled', which is set to true on passing any of
these two conditions.
It is quite unlikely behavior to have only one chip in Psafe or frequency
control disabled state. These two conditions are most likely to happen during
an OCC reset cycle which will occur across all chips.
Thanks and Regards,
Shilpa
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
On 05/05/2015 11:36 AM, Shilpasri G Bhat wrote:
Hi Preeti,
On 05/05/2015 09:21 AM, Preeti U Murthy wrote:
Hi Shilpa,
On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
max allowed frequency for that chip if the chip exceeds its power or
temperature limits. As Pmax capping is a chip level condition report
this throttling behavior at chip level and also do not set the global
'throttled' on Pmax capping instead set the per-chip throttled
variable. Report unthrottling if Pmax is restored after throttling.
This patch adds a structure to store chip id and throttled state of
the chip.
Signed-off-by: Shilpasri G Bhat shilpa.b...@linux.vnet.ibm.com
---
drivers/cpufreq/powernv-cpufreq.c | 59
---
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c
b/drivers/cpufreq/powernv-cpufreq.c
index ebef0d8..d0c18c9 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -27,6 +27,7 @@
#include linux/smp.h
#include linux/of.h
#include linux/reboot.h
+#include linux/slab.h
#include asm/cputhreads.h
#include asm/firmware.h
@@ -42,6 +43,13 @@
static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
static bool rebooting, throttled;
+static struct chip {
+ unsigned int id;
+ bool throttled;
+} *chips;
+
+static int nr_chips;
+
/*
* Note: The set of pstates consists of contiguous integers, the
* smallest of which is indicated by powernv_pstate_info.min, the
@@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
static void powernv_cpufreq_throttle_check(unsigned int cpu)
{
unsigned long pmsr;
- int pmsr_pmax, pmsr_lp;
+ int pmsr_pmax, pmsr_lp, i;
pmsr = get_pmspr(SPRN_PMSR);
+ for (i = 0; i nr_chips; i++)
+ if (chips[i].id == cpu_to_chip_id(cpu))
+ break;
+
/* Check for Pmax Capping */
pmsr_pmax = (s8)PMSR_MAX(pmsr);
if (pmsr_pmax != powernv_pstate_info.max) {
- throttled = true;
- pr_info(CPU %d Pmax is reduced to %d\n, cpu, pmsr_pmax);
- pr_info(Max allowed Pstate is capped\n);
+ if (chips[i].throttled)
+ goto next;
+ chips[i].throttled = true;
+ pr_info(CPU %d on Chip %u has Pmax reduced to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
+ } else if (chips[i].throttled) {
+ chips[i].throttled = false;
Is this check on pmax sufficient to indicate that the chip is unthrottled ?
Unthrottling due to Pmax uncapping here is specific to a chip. So it is
sufficient to decide throttling/unthrottling when OCC is active for that chip.
Ok then we can perhaps exit after detecting unthrottling here.
+ pr_info(CPU %d on Chip %u has Pmax restored to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
}
/*
* Check for Psafe by reading LocalPstate
* or check if Psafe_mode_active is set in PMSR.
*/
+next:
pmsr_lp = (s8)PMSR_LP(pmsr);
if ((pmsr_lp powernv_pstate_info.min) ||
(pmsr PMSR_PSAFE_ENABLE)) {
@@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
.attr = powernv_cpu_freq_attr,
What about the situation where although occ is active, this particular
chip has been throttled and we end up repeatedly reporting pstate set
to safe and frequency control disabled from OS ? Should we not have a
check on (chips[i].throttled) before reporting an anomaly for these two
scenarios as well just like you have for pmsr_pmax ?
We will not have Psafe and frequency control disabled repeatedly printed
because of global variable 'throttled', which is set to true on passing any of
these two conditions.
It is quite unlikely behavior to have only one chip in Psafe or frequency
control disabled state. These two conditions are most likely to happen during
an OCC reset cycle which will occur across all chips.
Let us then add a comment to indicate that Psafe and frequency control
disabled conditions will fail *only if OCC is inactive* and not
otherwise and that this is a system wide phenomenon.
Regards
Preeti U Murthy
Thanks and Regards,
Shilpa
___
Linuxppc-dev mailing list
linuxppc-...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
Hi Shilpa,
On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
> The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
> max allowed frequency for that chip if the chip exceeds its power or
> temperature limits. As Pmax capping is a chip level condition report
> this throttling behavior at chip level and also do not set the global
> 'throttled' on Pmax capping instead set the per-chip throttled
> variable. Report unthrottling if Pmax is restored after throttling.
>
> This patch adds a structure to store chip id and throttled state of
> the chip.
>
> Signed-off-by: Shilpasri G Bhat
> ---
> drivers/cpufreq/powernv-cpufreq.c | 59
> ---
> 1 file changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c
> b/drivers/cpufreq/powernv-cpufreq.c
> index ebef0d8..d0c18c9 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -42,6 +43,13 @@
> static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
> static bool rebooting, throttled;
>
> +static struct chip {
> + unsigned int id;
> + bool throttled;
> +} *chips;
> +
> +static int nr_chips;
> +
> /*
> * Note: The set of pstates consists of contiguous integers, the
> * smallest of which is indicated by powernv_pstate_info.min, the
> @@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
> static void powernv_cpufreq_throttle_check(unsigned int cpu)
> {
> unsigned long pmsr;
> - int pmsr_pmax, pmsr_lp;
> + int pmsr_pmax, pmsr_lp, i;
>
> pmsr = get_pmspr(SPRN_PMSR);
>
> + for (i = 0; i < nr_chips; i++)
> + if (chips[i].id == cpu_to_chip_id(cpu))
> + break;
> +
> /* Check for Pmax Capping */
> pmsr_pmax = (s8)PMSR_MAX(pmsr);
> if (pmsr_pmax != powernv_pstate_info.max) {
> - throttled = true;
> - pr_info("CPU %d Pmax is reduced to %d\n", cpu, pmsr_pmax);
> - pr_info("Max allowed Pstate is capped\n");
> + if (chips[i].throttled)
> + goto next;
> + chips[i].throttled = true;
> + pr_info("CPU %d on Chip %u has Pmax reduced to %d\n", cpu,
> + chips[i].id, pmsr_pmax);
> + } else if (chips[i].throttled) {
> + chips[i].throttled = false;
Is this check on pmax sufficient to indicate that the chip is unthrottled ?
> + pr_info("CPU %d on Chip %u has Pmax restored to %d\n", cpu,
> + chips[i].id, pmsr_pmax);
> }
>
> /*
>* Check for Psafe by reading LocalPstate
>* or check if Psafe_mode_active is set in PMSR.
>*/
> +next:
> pmsr_lp = (s8)PMSR_LP(pmsr);
> if ((pmsr_lp < powernv_pstate_info.min) ||
> (pmsr & PMSR_PSAFE_ENABLE)) {
> @@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
> .attr = powernv_cpu_freq_attr,
What about the situation where although occ is active, this particular
chip has been throttled and we end up repeatedly reporting "pstate set
to safe" and "frequency control disabled from OS" ? Should we not have a
check on (chips[i].throttled) before reporting an anomaly for these two
scenarios as well just like you have for pmsr_pmax ?
Regards
Preeti U Murthy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/6] cpufreq: poowernv: Handle throttling due to Pmax capping at chip level
Hi Shilpa,
On 05/04/2015 02:24 PM, Shilpasri G Bhat wrote:
The On-Chip-Controller(OCC) can throttle cpu frequency by reducing the
max allowed frequency for that chip if the chip exceeds its power or
temperature limits. As Pmax capping is a chip level condition report
this throttling behavior at chip level and also do not set the global
'throttled' on Pmax capping instead set the per-chip throttled
variable. Report unthrottling if Pmax is restored after throttling.
This patch adds a structure to store chip id and throttled state of
the chip.
Signed-off-by: Shilpasri G Bhat shilpa.b...@linux.vnet.ibm.com
---
drivers/cpufreq/powernv-cpufreq.c | 59
---
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c
b/drivers/cpufreq/powernv-cpufreq.c
index ebef0d8..d0c18c9 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -27,6 +27,7 @@
#include linux/smp.h
#include linux/of.h
#include linux/reboot.h
+#include linux/slab.h
#include asm/cputhreads.h
#include asm/firmware.h
@@ -42,6 +43,13 @@
static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
static bool rebooting, throttled;
+static struct chip {
+ unsigned int id;
+ bool throttled;
+} *chips;
+
+static int nr_chips;
+
/*
* Note: The set of pstates consists of contiguous integers, the
* smallest of which is indicated by powernv_pstate_info.min, the
@@ -301,22 +309,33 @@ static inline unsigned int get_nominal_index(void)
static void powernv_cpufreq_throttle_check(unsigned int cpu)
{
unsigned long pmsr;
- int pmsr_pmax, pmsr_lp;
+ int pmsr_pmax, pmsr_lp, i;
pmsr = get_pmspr(SPRN_PMSR);
+ for (i = 0; i nr_chips; i++)
+ if (chips[i].id == cpu_to_chip_id(cpu))
+ break;
+
/* Check for Pmax Capping */
pmsr_pmax = (s8)PMSR_MAX(pmsr);
if (pmsr_pmax != powernv_pstate_info.max) {
- throttled = true;
- pr_info(CPU %d Pmax is reduced to %d\n, cpu, pmsr_pmax);
- pr_info(Max allowed Pstate is capped\n);
+ if (chips[i].throttled)
+ goto next;
+ chips[i].throttled = true;
+ pr_info(CPU %d on Chip %u has Pmax reduced to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
+ } else if (chips[i].throttled) {
+ chips[i].throttled = false;
Is this check on pmax sufficient to indicate that the chip is unthrottled ?
+ pr_info(CPU %d on Chip %u has Pmax restored to %d\n, cpu,
+ chips[i].id, pmsr_pmax);
}
/*
* Check for Psafe by reading LocalPstate
* or check if Psafe_mode_active is set in PMSR.
*/
+next:
pmsr_lp = (s8)PMSR_LP(pmsr);
if ((pmsr_lp powernv_pstate_info.min) ||
(pmsr PMSR_PSAFE_ENABLE)) {
@@ -414,6 +433,33 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
.attr = powernv_cpu_freq_attr,
What about the situation where although occ is active, this particular
chip has been throttled and we end up repeatedly reporting pstate set
to safe and frequency control disabled from OS ? Should we not have a
check on (chips[i].throttled) before reporting an anomaly for these two
scenarios as well just like you have for pmsr_pmax ?
Regards
Preeti U Murthy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
