From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5487643BAC; Fri, 1 Mar 2024 04:34:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 20610402CD; Fri, 1 Mar 2024 04:34:00 +0100 (CET) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by mails.dpdk.org (Postfix) with ESMTP id 6E1534025C for ; Fri, 1 Mar 2024 04:33:58 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.88.214]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4TmDCb4KB7z1Q9Zv; Fri, 1 Mar 2024 11:31:39 +0800 (CST) Received: from kwepemm600004.china.huawei.com (unknown [7.193.23.242]) by mail.maildlp.com (Postfix) with ESMTPS id 69FB81A016C; Fri, 1 Mar 2024 11:33:56 +0800 (CST) Received: from [10.67.121.59] (10.67.121.59) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Fri, 1 Mar 2024 11:33:55 +0800 Message-ID: <887849f8-f8ab-2538-de1b-818b24898198@huawei.com> Date: Fri, 1 Mar 2024 11:33:55 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [RFC PATCH 2/2] power: refactor uncore power management library To: Sivaprasad Tummala CC: , , , , , , , , References: <20240220153326.6236-1-sivaprasad.tummala@amd.com> <20240220153326.6236-4-sivaprasad.tummala@amd.com> From: "lihuisong (C)" In-Reply-To: <20240220153326.6236-4-sivaprasad.tummala@amd.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.59] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemm600004.china.huawei.com (7.193.23.242) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 > --- > 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); \ > + }) > + <...>