From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Francisco Jerez <currojerez@riseup.net>
Cc: linux-pm@vger.kernel.org, intel-gfx@lists.freedesktop.org,
"Pandruvada, Srinivas" <srinivas.pandruvada@intel.com>,
"Vivi, Rodrigo" <rodrigo.vivi@intel.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 04/10] Revert "cpufreq: intel_pstate: Drop ->update_util from pstate_funcs"
Date: Thu, 19 Mar 2020 11:45:49 +0100 [thread overview]
Message-ID: <1617795.m7tDuoAjBp@kreacher> (raw)
In-Reply-To: <20200310214203.26459-5-currojerez@riseup.net>
On Tuesday, March 10, 2020 10:41:57 PM CET Francisco Jerez wrote:
> This reverts commit c4f3f70cacba2fa19545389a12d09b606d2ad1cf. A
> future commit will introduce a new update_util implementation, so the
> pstate_funcs table entry is going to be useful.
This basically means that you want to introduce a new scaling algorithm.
In my view that needs to be exposed via scaling_governor so users can
switch over between this and the existing ones (powersave and performance).
That would require the cpufreq core to be updated somewhat to recognize
an additional CPUFREQ_POLICY_ value, but that should be perfectly doable.
And ->
> Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> ---
> drivers/cpufreq/intel_pstate.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 7fa869004cf0..8cb5bf419b40 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -277,6 +277,7 @@ static struct cpudata **all_cpu_data;
> * @get_scaling: Callback to get frequency scaling factor
> * @get_val: Callback to convert P state to actual MSR write value
> * @get_vid: Callback to get VID data for Atom platforms
> + * @update_util: Active mode utilization update callback.
> *
> * Core and Atom CPU models have different way to get P State limits. This
> * structure is used to store those callbacks.
> @@ -290,6 +291,8 @@ struct pstate_funcs {
> int (*get_aperf_mperf_shift)(void);
> u64 (*get_val)(struct cpudata*, int pstate);
> void (*get_vid)(struct cpudata *);
> + void (*update_util)(struct update_util_data *data, u64 time,
> + unsigned int flags);
> };
>
> static struct pstate_funcs pstate_funcs __read_mostly;
> @@ -1877,6 +1880,7 @@ static struct pstate_funcs core_funcs = {
> .get_turbo = core_get_turbo_pstate,
> .get_scaling = core_get_scaling,
> .get_val = core_get_val,
> + .update_util = intel_pstate_update_util,
> };
>
> static const struct pstate_funcs silvermont_funcs = {
> @@ -1887,6 +1891,7 @@ static const struct pstate_funcs silvermont_funcs = {
> .get_val = atom_get_val,
> .get_scaling = silvermont_get_scaling,
> .get_vid = atom_get_vid,
> + .update_util = intel_pstate_update_util,
> };
>
> static const struct pstate_funcs airmont_funcs = {
> @@ -1897,6 +1902,7 @@ static const struct pstate_funcs airmont_funcs = {
> .get_val = atom_get_val,
> .get_scaling = airmont_get_scaling,
> .get_vid = atom_get_vid,
> + .update_util = intel_pstate_update_util,
> };
>
> static const struct pstate_funcs knl_funcs = {
> @@ -1907,6 +1913,7 @@ static const struct pstate_funcs knl_funcs = {
> .get_aperf_mperf_shift = knl_get_aperf_mperf_shift,
> .get_scaling = core_get_scaling,
> .get_val = core_get_val,
> + .update_util = intel_pstate_update_util,
> };
>
> #define ICPU(model, policy) \
> @@ -2013,9 +2020,7 @@ static void intel_pstate_set_update_util_hook(unsigned int cpu_num)
> /* Prevent intel_pstate_update_util() from using stale data. */
> cpu->sample.time = 0;
> cpufreq_add_update_util_hook(cpu_num, &cpu->update_util,
> - (hwp_active ?
> - intel_pstate_update_util_hwp :
> - intel_pstate_update_util));
-> it should be possible to extend this code to install an update_util matching
the scaling algo chosen by the user.
> + pstate_funcs.update_util);
> cpu->update_util_set = true;
> }
>
> @@ -2584,6 +2589,7 @@ static void __init copy_cpu_funcs(struct pstate_funcs *funcs)
> pstate_funcs.get_scaling = funcs->get_scaling;
> pstate_funcs.get_val = funcs->get_val;
> pstate_funcs.get_vid = funcs->get_vid;
> + pstate_funcs.update_util = funcs->update_util;
> pstate_funcs.get_aperf_mperf_shift = funcs->get_aperf_mperf_shift;
> }
>
> @@ -2750,8 +2756,11 @@ static int __init intel_pstate_init(void)
> id = x86_match_cpu(hwp_support_ids);
> if (id) {
> copy_cpu_funcs(&core_funcs);
> - if (!no_hwp) {
> + if (no_hwp) {
> + pstate_funcs.update_util = intel_pstate_update_util;
> + } else {
> hwp_active++;
> + pstate_funcs.update_util = intel_pstate_update_util_hwp;
> hwp_mode_bdw = id->driver_data;
> intel_pstate.attr = hwp_cpufreq_attrs;
> goto hwp_cpu_matched;
>
next prev parent reply other threads:[~2020-03-19 10:45 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-10 21:41 [RFC] GPU-bound energy efficiency improvements for the intel_pstate driver (v2) Francisco Jerez
2020-03-10 21:41 ` [PATCH 01/10] PM: QoS: Add CPU_RESPONSE_FREQUENCY global PM QoS limit Francisco Jerez
2020-03-11 12:42 ` Peter Zijlstra
2020-03-11 19:23 ` Francisco Jerez
2020-03-11 19:23 ` [PATCHv2 " Francisco Jerez
2020-03-19 10:25 ` Rafael J. Wysocki
2020-03-10 21:41 ` [PATCH 02/10] drm/i915: Adjust PM QoS response frequency based on GPU load Francisco Jerez
2020-03-10 22:26 ` [Intel-gfx] " Chris Wilson
2020-03-11 0:34 ` Francisco Jerez
2020-03-18 19:42 ` Francisco Jerez
2020-03-20 2:46 ` Francisco Jerez
2020-03-20 10:06 ` Chris Wilson
2020-03-11 10:00 ` Tvrtko Ursulin
2020-03-11 10:21 ` Chris Wilson
2020-03-11 19:54 ` Francisco Jerez
2020-03-12 11:52 ` Tvrtko Ursulin
2020-03-13 7:39 ` Francisco Jerez
2020-03-16 20:54 ` Francisco Jerez
2020-03-10 21:41 ` [PATCH 03/10] OPTIONAL: drm/i915: Expose PM QoS control parameters via debugfs Francisco Jerez
2020-03-10 21:41 ` [PATCH 04/10] Revert "cpufreq: intel_pstate: Drop ->update_util from pstate_funcs" Francisco Jerez
2020-03-19 10:45 ` Rafael J. Wysocki [this message]
2020-03-10 21:41 ` [PATCH 05/10] cpufreq: intel_pstate: Implement VLP controller statistics and status calculation Francisco Jerez
2020-03-19 11:06 ` Rafael J. Wysocki
2020-03-10 21:41 ` [PATCH 06/10] cpufreq: intel_pstate: Implement VLP controller target P-state range estimation Francisco Jerez
2020-03-19 11:12 ` Rafael J. Wysocki
2020-03-10 21:42 ` [PATCH 07/10] cpufreq: intel_pstate: Implement VLP controller for HWP parts Francisco Jerez
2020-03-17 23:59 ` Pandruvada, Srinivas
2020-03-18 19:51 ` Francisco Jerez
2020-03-18 20:10 ` Pandruvada, Srinivas
2020-03-18 20:22 ` Francisco Jerez
2020-03-23 20:13 ` Pandruvada, Srinivas
2020-03-10 21:42 ` [PATCH 08/10] cpufreq: intel_pstate: Enable VLP controller based on ACPI FADT profile and CPUID Francisco Jerez
2020-03-19 11:20 ` Rafael J. Wysocki
2020-03-10 21:42 ` [PATCH 09/10] OPTIONAL: cpufreq: intel_pstate: Add tracing of VLP controller status Francisco Jerez
2020-03-10 21:42 ` [PATCH 10/10] OPTIONAL: cpufreq: intel_pstate: Expose VLP controller parameters via debugfs Francisco Jerez
2020-03-11 2:35 ` [RFC] GPU-bound energy efficiency improvements for the intel_pstate driver (v2) Pandruvada, Srinivas
2020-03-11 3:55 ` Francisco Jerez
2020-03-23 23:29 ` Pandruvada, Srinivas
2020-03-24 0:23 ` Francisco Jerez
2020-03-24 19:16 ` Francisco Jerez
2020-03-24 20:03 ` Pandruvada, Srinivas
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=1617795.m7tDuoAjBp@kreacher \
--to=rjw@rjwysocki.net \
--cc=currojerez@riseup.net \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-pm@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rodrigo.vivi@intel.com \
--cc=srinivas.pandruvada@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;
as well as URLs for NNTP newsgroup(s).