From: "lihuisong (C)" <lihuisong@huawei.com>
To: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
Cc: <dev@dpdk.org>, <david.hunt@intel.com>,
<anatoly.burakov@intel.com>, <radu.nicolau@intel.com>,
<jerinj@marvell.com>, <cristian.dumitrescu@intel.com>,
<konstantin.ananyev@huawei.com>, <ferruh.yigit@amd.com>,
<gakhil@marvell.com>
Subject: Re: [RFC PATCH 2/2] power: refactor uncore power management library
Date: Fri, 1 Mar 2024 11:33:55 +0800 [thread overview]
Message-ID: <887849f8-f8ab-2538-de1b-818b24898198@huawei.com> (raw)
In-Reply-To: <20240220153326.6236-4-sivaprasad.tummala@amd.com>
Hi,
在 2024/2/20 23:33, Sivaprasad Tummala 写道:
> This patch refactors the power management library, addressing uncore
> power management. The primary changes involve the creation of dedicated
> directories for each driver within 'drivers/power/uncore/*'. The
> adjustment of meson.build files enables the selective activation
> of individual drivers.
+1 to discriminate core and uncore.
>
> This refactor significantly improves code organization, enhances
> clarity and boosts maintainability. It lays the foundation for more
> focused development on individual drivers and facilitates seamless
> integration of future enhancements, particularly the AMD uncore driver.
>
> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
> ---
> drivers/power/meson.build | 1 +
> drivers/power/uncore/intel/meson.build | 9 +
> .../power/uncore/intel}/power_intel_uncore.c | 15 ++
> .../power/uncore/intel}/power_intel_uncore.h | 0
> drivers/power/uncore/meson.build | 8 +
> lib/power/meson.build | 1 -
> lib/power/rte_power_uncore.c | 163 +++++++-----------
> lib/power/rte_power_uncore.h | 150 ++++++++++++++--
> lib/power/version.map | 1 +
> 9 files changed, 236 insertions(+), 112 deletions(-)
> create mode 100644 drivers/power/uncore/intel/meson.build
> rename {lib/power => drivers/power/uncore/intel}/power_intel_uncore.c (95%)
> rename {lib/power => drivers/power/uncore/intel}/power_intel_uncore.h (100%)
How about remove 'power' in "power_intel_uncore.c"
> create mode 100644 drivers/power/uncore/meson.build
>
> diff --git a/drivers/power/meson.build b/drivers/power/meson.build
> index 7d9034c7ac..0803e99027 100644
> --- a/drivers/power/meson.build
> +++ b/drivers/power/meson.build
> @@ -3,6 +3,7 @@
>
> drivers = [
> 'core',
> + 'uncore',
> ]
>
> std_deps = ['power']
> diff --git a/drivers/power/uncore/intel/meson.build b/drivers/power/uncore/intel/meson.build
> new file mode 100644
> index 0000000000..187ab15aec
> --- /dev/null
> +++ b/drivers/power/uncore/intel/meson.build
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2017 Intel Corporation
> +# Copyright(c) 2024 AMD Limited
> +
> +sources = files('power_intel_uncore.c')
> +
> +headers = files('power_intel_uncore.h')
> +
> +deps += ['power']
> diff --git a/lib/power/power_intel_uncore.c b/drivers/power/uncore/intel/power_intel_uncore.c
> similarity index 95%
> rename from lib/power/power_intel_uncore.c
> rename to drivers/power/uncore/intel/power_intel_uncore.c
> index 3ce8fccec2..3af4cc3bc7 100644
> --- a/lib/power/power_intel_uncore.c
> +++ b/drivers/power/uncore/intel/power_intel_uncore.c
> @@ -476,3 +476,18 @@ power_intel_uncore_get_num_dies(unsigned int pkg)
>
> return count;
> }
> +
> +static struct rte_power_uncore_ops intel_uncore_ops = {
> + .init = power_intel_uncore_init,
> + .exit = power_intel_uncore_exit,
> + .get_avail_freqs = power_intel_uncore_freqs,
> + .get_num_pkgs = power_intel_uncore_get_num_pkgs,
> + .get_num_dies = power_intel_uncore_get_num_dies,
> + .get_num_freqs = power_intel_uncore_get_num_freqs,
> + .get_freq = power_get_intel_uncore_freq,
> + .set_freq = power_set_intel_uncore_freq,
> + .freq_max = power_intel_uncore_freq_max,
> + .freq_min = power_intel_uncore_freq_min,
> +};
> +
> +RTE_POWER_REGISTER_UNCORE_OPS(intel_uncore_ops);
<...>
> +
> +/** Structure defining uncore power operations structure */
> +struct rte_power_uncore_ops {
> + uint8_t status; /**< ops register status. */
> + enum rte_uncore_power_mgmt_env env; /**< power mgmt env. */
> + rte_power_uncore_init_t init; /**< Initialize power management. */
> + rte_power_uncore_exit_t exit; /**< Exit power management. */
> + rte_power_uncore_get_num_pkgs_t get_num_pkgs;
> + rte_power_uncore_get_num_dies_t get_num_dies;
> + rte_power_uncore_get_num_freqs_t get_num_freqs; /**< Number of available frequencies. */
> + rte_power_uncore_freqs_t get_avail_freqs; /**< Get the available frequencies. */
> + rte_power_get_uncore_freq_t get_freq; /**< Get frequency index. */
> + rte_power_set_uncore_freq_t set_freq; /**< Set frequency index. */
> + rte_power_uncore_freq_change_t freq_max; /**< Scale up frequency to highest. */
> + rte_power_uncore_freq_change_t freq_min; /**< Scale up frequency to lowest. */
> +} __rte_cache_aligned;
For all core drivers (cpufreq), they all basically follow the ACPI
specification.
So libray can extract a common ops for all core DVFS driver.
AFAIS, there is only one uncore driver in kernel, namely intel uncore
driver.
But there is not an unify specification to control uncore frequency
scaling(UFS) in kernel.
That is to say, every chip manufacturers can implement their uncore
driver as themselves request.
As a result, there is different system interface for userspace between
manufacturer.
So I am not sure if this new extracted rte_power_uncore_ops sturcture is
very common for all uncore drivers in future.
> +
> +/**
> + * Register power uncore frequency operations.
> + * @param ops
> + * Pointer to an ops structure to register.
> + * @return
> + * - >=0: Success; return the index of the ops struct in the table.
> + * - -EINVAL - error while registering ops struct.
> + */
> +__rte_internal
> +int rte_power_register_uncore_ops(const struct rte_power_uncore_ops *ops);
> +
> +/**
> + * Macro to statically register the ops of an uncore driver.
> + */
> +#define RTE_POWER_REGISTER_UNCORE_OPS(ops) \
> + (RTE_INIT(power_hdlr_init_uncore_##ops) \
> + { \
> + rte_power_register_uncore_ops(&ops); \
> + })
> +
<...>
next prev parent reply other threads:[~2024-03-01 3:34 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-20 15:33 [RFC PATCH 0/2] power: refactor " Sivaprasad Tummala
2024-02-20 15:33 ` Sivaprasad Tummala
2024-02-20 15:33 ` [RFC PATCH 1/2] power: refactor core " Sivaprasad Tummala
2024-02-27 16:18 ` Ferruh Yigit
2024-02-29 7:10 ` Tummala, Sivaprasad
2024-02-28 12:51 ` Ferruh Yigit
2024-03-01 2:56 ` lihuisong (C)
2024-03-01 10:39 ` Hunt, David
2024-03-05 4:35 ` Tummala, Sivaprasad
2024-02-20 15:33 ` [RFC PATCH 2/2] power: refactor uncore " Sivaprasad Tummala
2024-03-01 3:33 ` lihuisong (C) [this message]
2024-03-01 6:06 ` Tummala, Sivaprasad
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=887849f8-f8ab-2538-de1b-818b24898198@huawei.com \
--to=lihuisong@huawei.com \
--cc=anatoly.burakov@intel.com \
--cc=cristian.dumitrescu@intel.com \
--cc=david.hunt@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=gakhil@marvell.com \
--cc=jerinj@marvell.com \
--cc=konstantin.ananyev@huawei.com \
--cc=radu.nicolau@intel.com \
--cc=sivaprasad.tummala@amd.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).