* [PATCH] ACPI: update user_policy.max when _PPC updated
@ 2013-06-05 0:52 Joe Jin
2013-06-05 20:40 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Joe Jin @ 2013-06-05 0:52 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar
Cc: Neil Gu, cpufreq, linux-pm, linux-kernel@vger.kernel.org
When _PPC changed dynamically the user_policy.max will not be updated,
this prevent CPU run on the highest frequency.
Signed-off-by: Joe Jin <joe.jin@oracle.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index e854582..e01aa7d 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
{
int ret;
+ unsigned int saved = (unsigned int)pr->performance_platform_limit;
if (ignore_ppc) {
/*
@@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
}
if (ret < 0)
return (ret);
- else
+ else {
+ unsigned int ppc = (unsigned int)pr->performance_platform_limit;
+
+ if (saved != ppc) {
+ struct cpufreq_policy *policy;
+
+ policy = cpufreq_cpu_get(pr->id);
+ if (likely(policy))
+ policy->user_policy.max =
+ pr->performance->states[ppc].
+ core_frequency * 1000;
+ cpufreq_cpu_put(policy);
+ }
+
return cpufreq_update_policy(pr->id);
+ }
}
int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: update user_policy.max when _PPC updated
2013-06-05 0:52 [PATCH] ACPI: update user_policy.max when _PPC updated Joe Jin
@ 2013-06-05 20:40 ` Rafael J. Wysocki
2013-06-06 0:27 ` Joe Jin
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2013-06-05 20:40 UTC (permalink / raw)
To: Joe Jin
Cc: Viresh Kumar, Neil Gu, cpufreq, linux-pm,
linux-kernel@vger.kernel.org
On Wednesday, June 05, 2013 08:52:52 AM Joe Jin wrote:
> When _PPC changed dynamically the user_policy.max will not be updated,
> this prevent CPU run on the highest frequency.
Why should the user setting be always related to the current maximum available
frequency? What if the user sets the limit for power capping purposes?
Rafael
> Signed-off-by: Joe Jin <joe.jin@oracle.com>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index e854582..e01aa7d 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
> int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> {
> int ret;
> + unsigned int saved = (unsigned int)pr->performance_platform_limit;
>
> if (ignore_ppc) {
> /*
> @@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> }
> if (ret < 0)
> return (ret);
> - else
> + else {
> + unsigned int ppc = (unsigned int)pr->performance_platform_limit;
> +
> + if (saved != ppc) {
> + struct cpufreq_policy *policy;
> +
> + policy = cpufreq_cpu_get(pr->id);
> + if (likely(policy))
> + policy->user_policy.max =
> + pr->performance->states[ppc].
> + core_frequency * 1000;
> + cpufreq_cpu_put(policy);
> + }
> +
> return cpufreq_update_policy(pr->id);
> + }
> }
>
> int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: update user_policy.max when _PPC updated
2013-06-05 20:40 ` Rafael J. Wysocki
@ 2013-06-06 0:27 ` Joe Jin
2013-06-06 11:06 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Joe Jin @ 2013-06-06 0:27 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Viresh Kumar, Neil Gu, cpufreq, linux-pm,
linux-kernel@vger.kernel.org
On 06/06/13 04:40, Rafael J. Wysocki wrote:
> On Wednesday, June 05, 2013 08:52:52 AM Joe Jin wrote:
>> When _PPC changed dynamically the user_policy.max will not be updated,
>> this prevent CPU run on the highest frequency.
>
> Why should the user setting be always related to the current maximum available
> frequency? What if the user sets the limit for power capping purposes?
cpufreq_update_policy() get policy->max from user_policy.max:
1782 int cpufreq_update_policy(unsigned int cpu)
1783 {
[...]
1800 policy.min = data->user_policy.min;
1801 policy.max = data->user_policy.max;
1802 policy.policy = data->user_policy.policy;
1803 policy.governor = data->user_policy.governor;
[...]
1819 ret = __cpufreq_set_policy(data, &policy);
[...]
/sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max
and user_policy->max, when update it, so I think _PPC changes also need
to update these two?
Thanks,
Joe
>
> Rafael
>
>
>> Signed-off-by: Joe Jin <joe.jin@oracle.com>
>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
>> Cc: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>> drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
>> index e854582..e01aa7d 100644
>> --- a/drivers/acpi/processor_perflib.c
>> +++ b/drivers/acpi/processor_perflib.c
>> @@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
>> int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
>> {
>> int ret;
>> + unsigned int saved = (unsigned int)pr->performance_platform_limit;
>>
>> if (ignore_ppc) {
>> /*
>> @@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
>> }
>> if (ret < 0)
>> return (ret);
>> - else
>> + else {
>> + unsigned int ppc = (unsigned int)pr->performance_platform_limit;
>> +
>> + if (saved != ppc) {
>> + struct cpufreq_policy *policy;
>> +
>> + policy = cpufreq_cpu_get(pr->id);
>> + if (likely(policy))
>> + policy->user_policy.max =
>> + pr->performance->states[ppc].
>> + core_frequency * 1000;
>> + cpufreq_cpu_put(policy);
>> + }
>> +
>> return cpufreq_update_policy(pr->id);
>> + }
>> }
>>
>> int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
>>
--
Oracle <http://www.oracle.com>
Joe Jin | Software Development Senior Manager | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: update user_policy.max when _PPC updated
2013-06-06 0:27 ` Joe Jin
@ 2013-06-06 11:06 ` Rafael J. Wysocki
2013-06-06 13:29 ` Joe Jin
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2013-06-06 11:06 UTC (permalink / raw)
To: Joe Jin
Cc: Viresh Kumar, Neil Gu, cpufreq, linux-pm,
linux-kernel@vger.kernel.org
On Thursday, June 06, 2013 08:27:08 AM Joe Jin wrote:
> On 06/06/13 04:40, Rafael J. Wysocki wrote:
> > On Wednesday, June 05, 2013 08:52:52 AM Joe Jin wrote:
> >> When _PPC changed dynamically the user_policy.max will not be updated,
> >> this prevent CPU run on the highest frequency.
> >
> > Why should the user setting be always related to the current maximum available
> > frequency? What if the user sets the limit for power capping purposes?
>
> cpufreq_update_policy() get policy->max from user_policy.max:
>
> 1782 int cpufreq_update_policy(unsigned int cpu)
> 1783 {
> [...]
> 1800 policy.min = data->user_policy.min;
> 1801 policy.max = data->user_policy.max;
> 1802 policy.policy = data->user_policy.policy;
> 1803 policy.governor = data->user_policy.governor;
> [...]
> 1819 ret = __cpufreq_set_policy(data, &policy);
> [...]
>
> /sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max
> and user_policy->max, when update it, so I think _PPC changes also need
> to update these two?
Yes, if policy.max happens to be greater that the maximum available frequency,
then (and only then) it probably should be updated. It should never be bumped
up, though.
Thanks,
Rafael
> >> Signed-off-by: Joe Jin <joe.jin@oracle.com>
> >> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> >> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> >> ---
> >> drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
> >> 1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> >> index e854582..e01aa7d 100644
> >> --- a/drivers/acpi/processor_perflib.c
> >> +++ b/drivers/acpi/processor_perflib.c
> >> @@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
> >> int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> >> {
> >> int ret;
> >> + unsigned int saved = (unsigned int)pr->performance_platform_limit;
> >>
> >> if (ignore_ppc) {
> >> /*
> >> @@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> >> }
> >> if (ret < 0)
> >> return (ret);
> >> - else
> >> + else {
> >> + unsigned int ppc = (unsigned int)pr->performance_platform_limit;
> >> +
> >> + if (saved != ppc) {
> >> + struct cpufreq_policy *policy;
> >> +
> >> + policy = cpufreq_cpu_get(pr->id);
> >> + if (likely(policy))
> >> + policy->user_policy.max =
> >> + pr->performance->states[ppc].
> >> + core_frequency * 1000;
> >> + cpufreq_cpu_put(policy);
> >> + }
> >> +
> >> return cpufreq_update_policy(pr->id);
> >> + }
> >> }
> >>
> >> int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> >>
>
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: update user_policy.max when _PPC updated
2013-06-06 11:06 ` Rafael J. Wysocki
@ 2013-06-06 13:29 ` Joe Jin
2013-06-06 19:54 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Joe Jin @ 2013-06-06 13:29 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Viresh Kumar, Neil Gu, cpufreq, linux-pm,
linux-kernel@vger.kernel.org
On 06/06/13 19:06, Rafael J. Wysocki wrote:
> On Thursday, June 06, 2013 08:27:08 AM Joe Jin wrote:
>> On 06/06/13 04:40, Rafael J. Wysocki wrote:
>>> On Wednesday, June 05, 2013 08:52:52 AM Joe Jin wrote:
>>>> When _PPC changed dynamically the user_policy.max will not be updated,
>>>> this prevent CPU run on the highest frequency.
>>>
>>> Why should the user setting be always related to the current maximum available
>>> frequency? What if the user sets the limit for power capping purposes?
>>
>> cpufreq_update_policy() get policy->max from user_policy.max:
>>
>> 1782 int cpufreq_update_policy(unsigned int cpu)
>> 1783 {
>> [...]
>> 1800 policy.min = data->user_policy.min;
>> 1801 policy.max = data->user_policy.max;
>> 1802 policy.policy = data->user_policy.policy;
>> 1803 policy.governor = data->user_policy.governor;
>> [...]
>> 1819 ret = __cpufreq_set_policy(data, &policy);
>> [...]
>>
>> /sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max
>> and user_policy->max, when update it, so I think _PPC changes also need
>> to update these two?
>
> Yes, if policy.max happens to be greater that the maximum available frequency,
> then (and only then) it probably should be updated. It should never be bumped
> up, though.
Does this means if I enabled hardcap before kernel boot up, and later system brought
up and I disabled hardcap, I has to enlarge the max frequency manually?
Thanks,
Joe
>
> Thanks,
> Rafael
>
>
>>>> Signed-off-by: Joe Jin <joe.jin@oracle.com>
>>>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
>>>> Cc: Viresh Kumar <viresh.kumar@linaro.org>
>>>> ---
>>>> drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
>>>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
>>>> index e854582..e01aa7d 100644
>>>> --- a/drivers/acpi/processor_perflib.c
>>>> +++ b/drivers/acpi/processor_perflib.c
>>>> @@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
>>>> int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
>>>> {
>>>> int ret;
>>>> + unsigned int saved = (unsigned int)pr->performance_platform_limit;
>>>>
>>>> if (ignore_ppc) {
>>>> /*
>>>> @@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
>>>> }
>>>> if (ret < 0)
>>>> return (ret);
>>>> - else
>>>> + else {
>>>> + unsigned int ppc = (unsigned int)pr->performance_platform_limit;
>>>> +
>>>> + if (saved != ppc) {
>>>> + struct cpufreq_policy *policy;
>>>> +
>>>> + policy = cpufreq_cpu_get(pr->id);
>>>> + if (likely(policy))
>>>> + policy->user_policy.max =
>>>> + pr->performance->states[ppc].
>>>> + core_frequency * 1000;
>>>> + cpufreq_cpu_put(policy);
>>>> + }
>>>> +
>>>> return cpufreq_update_policy(pr->id);
>>>> + }
>>>> }
>>>>
>>>> int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
>>>>
>>
>>
>>
--
Oracle <http://www.oracle.com>
Joe Jin | Software Development Senior Manager | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: update user_policy.max when _PPC updated
2013-06-06 13:29 ` Joe Jin
@ 2013-06-06 19:54 ` Rafael J. Wysocki
2013-06-07 1:48 ` Joe Jin
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2013-06-06 19:54 UTC (permalink / raw)
To: Joe Jin
Cc: Viresh Kumar, Neil Gu, cpufreq, linux-pm,
linux-kernel@vger.kernel.org
On Thursday, June 06, 2013 09:29:27 PM Joe Jin wrote:
> On 06/06/13 19:06, Rafael J. Wysocki wrote:
> > On Thursday, June 06, 2013 08:27:08 AM Joe Jin wrote:
> >> On 06/06/13 04:40, Rafael J. Wysocki wrote:
> >>> On Wednesday, June 05, 2013 08:52:52 AM Joe Jin wrote:
> >>>> When _PPC changed dynamically the user_policy.max will not be updated,
> >>>> this prevent CPU run on the highest frequency.
> >>>
> >>> Why should the user setting be always related to the current maximum available
> >>> frequency? What if the user sets the limit for power capping purposes?
> >>
> >> cpufreq_update_policy() get policy->max from user_policy.max:
> >>
> >> 1782 int cpufreq_update_policy(unsigned int cpu)
> >> 1783 {
> >> [...]
> >> 1800 policy.min = data->user_policy.min;
> >> 1801 policy.max = data->user_policy.max;
> >> 1802 policy.policy = data->user_policy.policy;
> >> 1803 policy.governor = data->user_policy.governor;
> >> [...]
> >> 1819 ret = __cpufreq_set_policy(data, &policy);
> >> [...]
> >>
> >> /sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max
> >> and user_policy->max, when update it, so I think _PPC changes also need
> >> to update these two?
> >
> > Yes, if policy.max happens to be greater that the maximum available frequency,
> > then (and only then) it probably should be updated. It should never be bumped
> > up, though.
>
> Does this means if I enabled hardcap before kernel boot up, and later system brought
> up and I disabled hardcap, I has to enlarge the max frequency manually?
Do you mean you set a limit in the BIOS setup and the kernel changed that limit
on boot?
Rafael
> >>>> Signed-off-by: Joe Jin <joe.jin@oracle.com>
> >>>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> >>>> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> >>>> ---
> >>>> drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
> >>>> 1 file changed, 16 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> >>>> index e854582..e01aa7d 100644
> >>>> --- a/drivers/acpi/processor_perflib.c
> >>>> +++ b/drivers/acpi/processor_perflib.c
> >>>> @@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
> >>>> int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> >>>> {
> >>>> int ret;
> >>>> + unsigned int saved = (unsigned int)pr->performance_platform_limit;
> >>>>
> >>>> if (ignore_ppc) {
> >>>> /*
> >>>> @@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> >>>> }
> >>>> if (ret < 0)
> >>>> return (ret);
> >>>> - else
> >>>> + else {
> >>>> + unsigned int ppc = (unsigned int)pr->performance_platform_limit;
> >>>> +
> >>>> + if (saved != ppc) {
> >>>> + struct cpufreq_policy *policy;
> >>>> +
> >>>> + policy = cpufreq_cpu_get(pr->id);
> >>>> + if (likely(policy))
> >>>> + policy->user_policy.max =
> >>>> + pr->performance->states[ppc].
> >>>> + core_frequency * 1000;
> >>>> + cpufreq_cpu_put(policy);
> >>>> + }
> >>>> +
> >>>> return cpufreq_update_policy(pr->id);
> >>>> + }
> >>>> }
> >>>>
> >>>> int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> >>>>
> >>
> >>
> >>
>
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: update user_policy.max when _PPC updated
2013-06-06 19:54 ` Rafael J. Wysocki
@ 2013-06-07 1:48 ` Joe Jin
0 siblings, 0 replies; 7+ messages in thread
From: Joe Jin @ 2013-06-07 1:48 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Viresh Kumar, Neil Gu, cpufreq, linux-pm,
linux-kernel@vger.kernel.org
On 06/07/13 03:54, Rafael J. Wysocki wrote:
> Do you mean you set a limit in the BIOS setup and the kernel changed that limit
> on boot?
Sorry for the confusing.
The issue is when we disable hardcap before kernel boot up, after kernel bring
up, any changes of _PPC will update scaling_max_freq properly.
If we enable hardcap before kernel boot up, after kernel bring up, even we
disable it, scaling_max_freq does not be updated to max frequency, the max
frequency just up to the value when bring up.
Review related codes I found the limit came from user_policy.max, means when
we set user_policy.max to 1000MHZ when boot up, then any changes of _PPC could
not enlarge the scaling_max_freq, I think this is not as expected? please advise.
Thanks,
Joe
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-06-07 1:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-05 0:52 [PATCH] ACPI: update user_policy.max when _PPC updated Joe Jin
2013-06-05 20:40 ` Rafael J. Wysocki
2013-06-06 0:27 ` Joe Jin
2013-06-06 11:06 ` Rafael J. Wysocki
2013-06-06 13:29 ` Joe Jin
2013-06-06 19:54 ` Rafael J. Wysocki
2013-06-07 1:48 ` Joe Jin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).