public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
To: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>,
	linux-pm@vger.kernel.org, "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-acpi@vger.kernel.org,
	Kristen Carlson Accardi <kristen@linux.intel.com>,
	Len Brown <lenb@kernel.org>, Ethan Zhao <ethan.zhao@oracle.com>
Subject: Re: [PATCH RFC] intel_pstate: play well with frequency limits set by acpi
Date: Tue, 21 Jul 2015 19:37:43 +0300	[thread overview]
Message-ID: <55AE7557.5010605@yandex-team.ru> (raw)
In-Reply-To: <1437493051.2377.170.camel@spandruv-DESK3.jf.intel.com>

On 21.07.2015 18:37, Srinivas Pandruvada wrote:
> On Tue, 2015-07-21 at 13:25 +0300, Konstantin Khlebnikov wrote:
>> On 21.07.2015 00:08, Srinivas Pandruvada wrote:
>>> On Fri, 2015-07-17 at 07:36 +0300, Konstantin Khlebnikov wrote:
>>>> On Fri, Jul 17, 2015 at 1:08 AM, Srinivas Pandruvada
>>>> <srinivas.pandruvada@linux.intel.com> wrote:
>>>>> On Thu, 2015-07-16 at 21:17 +0300, Konstantin Khlebnikov wrote:
>>>>>> IPMI can control CPU P-states remotely: configuration is reported via
>>>>>> common ACPI interface (_PPC/_PSS/etc). This patch adds required minimal
>>>>>> support in intel_pstate to receive and use these P-state limits.
>>>>>>
>>>>>> * ignore limit of top state in _PPC: it lower than turbo boost frequency
>>>>>> * register intel_pstate in acpi-processor to get states from _PSS
>>>>>> * link acpi_processor_get_bios_limit: this adds attribute "bios_limit"
>>>>>>
>>>>>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>>>>> ---
>>>>>>    drivers/acpi/processor_perflib.c |    3 +-
>>>>>>    drivers/cpufreq/intel_pstate.c   |   57 ++++++++++++++++++++++++++++++++++++++
>>>>>>    2 files changed, 59 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
>>>>>> index cfc8aba72f86..781e328c9d5f 100644
>>>>>> --- a/drivers/acpi/processor_perflib.c
>>>>>> +++ b/drivers/acpi/processor_perflib.c
>>>>>> @@ -98,7 +98,8 @@ static int acpi_processor_ppc_notifier(struct notifier_block *nb,
>>>>>>
>>>>>>         ppc = (unsigned int)pr->performance_platform_limit;
>>>>>>
>>>>>> -     if (ppc >= pr->performance->state_count)
>>>>>> +     /* Ignore limit of top state: it lower than turbo boost frequency */
>>>>>> +     if (!ppc || ppc >= pr->performance->state_count)
>>>>> Why? Isn't the previous check enough?
>>>>
>>>> Zero _PPC state must be top performance state but as I see frequency in
>>>> _PSS is lower than maximum possible turbo frequency. So, in this case
>>>> intel_pstate cannnot get "100%"  for max bound even it there is no limit set.
>>>>
>>>> For example: I saw _PSS[0] = 2601 Mhz, PSS[1] = 2600 Mhz while turbo
>>>> state is 3400 Mhz.
>>>>
>>> Have you tested dynamic _PPC modification with acpi cpufreq with this
>>> change (after boot)? Suppose _PPC is changed from 3 to 0, then
>>> cpufreq_verify_within_limits will not be called to change to new max
>>> turbo performance state.
>>>
>>
>> I haven't checked that but as I see acpi_processor_ppc_notifier()
>> can only reduce maximum frequency. So, there should be no problem
>> in this case.
> No, it can also be used in both ways. Once reduced, it can increase as
> well. _PPC can be dynamically modified by BIOS to reduce and also to
> increase.

Well, in this case BIOS will trigger ACPI_PROCESSOR_NOTIFY_PERFORMANCE:
kernel evaluate new _PPC and call cpufreq_update_policy()
which set initial frequency min/max range according to user setup and
apply all limits after that. Initial policy->user_policy.min/max stay
unchanged. So, that dynamic modification works in both ways.

-- 
Konstantin

  reply	other threads:[~2015-07-21 16:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-16 18:17 [PATCH RFC] intel_pstate: play well with frequency limits set by acpi Konstantin Khlebnikov
2015-07-16 22:08 ` Srinivas Pandruvada
2015-07-17  4:36   ` Konstantin Khlebnikov
2015-07-20 21:08     ` Srinivas Pandruvada
2015-07-21 10:25       ` Konstantin Khlebnikov
2015-07-21 15:37         ` Srinivas Pandruvada
2015-07-21 16:37           ` Konstantin Khlebnikov [this message]
2015-07-21 18:53             ` Srinivas Pandruvada
2015-07-17  6:00 ` ethan zhao
2015-07-17  7:10   ` Konstantin Khlebnikov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55AE7557.5010605@yandex-team.ru \
    --to=khlebnikov@yandex-team.ru \
    --cc=ethan.zhao@oracle.com \
    --cc=koct9i@gmail.com \
    --cc=kristen@linux.intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=srinivas.pandruvada@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox