Re: [PATCH] ACPI: update user_policy.max when _PPC updated

From: Rafael J. Wysocki
Date: Thu Jun 06 2013 - 15:45:08 EST


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@xxxxxxxxxx>
> >>>> Cc: Rafael J. Wysocki <rjw@xxxxxxx>
> >>>> Cc: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> >>>> ---
> >>>> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/