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 A9321A0542; Tue, 4 Oct 2022 19:09:28 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5881540DDC; Tue, 4 Oct 2022 19:09:28 +0200 (CEST) Received: from wout1-smtp.messagingengine.com (wout1-smtp.messagingengine.com [64.147.123.24]) by mails.dpdk.org (Postfix) with ESMTP id 1992B40A79 for ; Tue, 4 Oct 2022 19:09:27 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 5A5A23200124; Tue, 4 Oct 2022 13:09:25 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Tue, 04 Oct 2022 13:09:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1664903364; x= 1664989764; bh=J0KEkisKeqZXh78PUziHmAt8xIzJXSu7lk7J1wB5Ngg=; b=k A3ZKfboaMeXEe6A5kmqOZmGBLRQR9fsws4TyGWogWW60LnZz9DVKLZ9IGpcYHNLr c9Q3l891HdI9wYmbNtzUpVvAhXUudUk5yfNcdNTOwd8TYhPYjlxaYPjxDa8FdD0H CoDEMINo3ORsArAHwB5sqnQTuqMYPFSjNlYmEvOQPRHTrt4xdxDOzTqWKW2iNG8f A689Dy1dsT7O+M/9FqpfafMHm4wxbwMvZcpE2Ne9GuE+qACNWqxuAQPhAbSw/I2k fnMO9psgh3LhKPBsryrtgwFVNL8Z25rDc6tUE1lgA5ciERZmWKVdIzZ7+B/X2YVe SDgraC+qWEsy1P14IhAtQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1664903364; x= 1664989764; bh=J0KEkisKeqZXh78PUziHmAt8xIzJXSu7lk7J1wB5Ngg=; b=r FoEcltl9Ap6hcqfvbZ5sMIijOiPU50VNPX/IS6+AMuBaxcvEmBPy61/FaNrLzx0E +OaQ8nF7B4lw7RnlfiVjcMEyYdbPyZ+jPGInyRqRm1E3/WbR6v7GfGjccZSj5wE2 YX0Zlgbtlw3iSXlsgmRYU+PEMHxJh2HUu8sHtB2KBLMCnquECpAcV4NtTiotcpY7 g3GuLPLaQtjIL1gcsm1YSMvBTo34eqrVd1VCdVsAJjUB55GaaAMyVl4jBblZks80 +6926UWnOzL1BucC5077plKkJaEqyI/zy1XNzCrqojLa8mRFNOEpn95XUE5tdPId CqkVn+QjkwpiYsw4+ZiIQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeeiuddguddtlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnhepgedttdeljeejgeffkeekkedtjeevtdehvedtkeeivdeuuedv ieduvdelveejueejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 4 Oct 2022 13:09:24 -0400 (EDT) From: Thomas Monjalon To: Tadhg Kearney Cc: dev@dpdk.org, david.hunt@intel.com, anatoly.burakov@intel.com, reshma.pattan@intel.com Subject: Re: [PATCH v7 1/3] power: add uncore frequency control API to the power library Date: Tue, 04 Oct 2022 19:09:22 +0200 Message-ID: <4956396.e8TTKsaY2g@thomas> In-Reply-To: <20220928133018.1583280-2-tadhg.kearney@intel.com> References: <20220928090636.1580647-1-tadhg.kearney@intel.com> <20220928133018.1583280-1-tadhg.kearney@intel.com> <20220928133018.1583280-2-tadhg.kearney@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" 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 28/09/2022 15:30, Tadhg Kearney: > Add API to allow uncore frequency adjustment. This is done through > manipulating related uncore frequency control sysfs entries to > adjust the minimum and maximum uncore frequency values. > Nine API's are being added that are all public and experimental. You cannot introduce an API without explaining what it is about. Maybe I'm an idiot but I don't know what is "uncore". I see it is explained in the documentation, but few words in the commit message would not be too much. At least you must say it for Linux on Intel, and which feature it is controlling. > +Uncore API > +---------- > + > +Abstract > +~~~~~~~~ > + > +Uncore is a term used by Intel to describe the functions of a microproce= ssor that are > +not in the core, but which must be closely connected to the core to achi= eve high performance; > +L3 cache, on-die memory controller, etc. > +Significant power savings can be achieved by reducing the uncore frequen= cy to its lowest value. So this is an Intel thing. > + > +The Linux kernel provides the driver =E2=80=9Cintel-uncore-frequency" to= control the uncore frequency limits > +for x86 platform. The driver is available from kernel version 5.6 and ab= ove. > +Also CONFIG_INTEL_UNCORE_FREQ_CONTROL will need to be enabled in the ker= nel, which was added in 5.6. > +This manipulates the contest of MSR 0x620, which sets min/max of the unc= ore for the SKU. It is correctly named "intel-uncore" in the Linux kernel. Why not having "Intel" in the DPDK feature name? > + > + > +API Overview for Uncore > +~~~~~~~~~~~~~~~~~~~~~~~ A blank line is missing here. > +* **Uncore Power Init**: Initialise uncore power, populate frequency arr= ay and record > + original min & max for pkg & die. > + > +* **Uncore Power Exit**: Exit uncore power, restoring original min & max= for pkg & die. > + > +* **Get Uncore Power Freq**: Get current uncore freq index for pkg & die. > + > +* **Set Uncore Power Freq**: Set min & max uncore freq index for pkg & d= ie (min and max will be the same). > + > +* **Uncore Power Max**: Set max uncore freq index for pkg & die. > + > +* **Uncore Power Min**: Set min uncore freq index for pkg & die. > + > +* **Get Num Freqs**: Get the number of frequencies in the index array. > + > +* **Get Num Pkgs**: Get the number of packages (CPUs) on the system. > + > +* **Get Num Dies**: Get the number of die's on a given package. Not sure what you are listing here. Are they functions? If you really want to keep a list, I suggest using a definition list available in RST syntax. If you want to provide an explanation easy to read, full sentences connecting things together would be better. > + > References > ---------- > =20 > diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_note= s/release_22_11.rst > index cb7677fd3c..5d3f815b54 100644 > --- a/doc/guides/rel_notes/release_22_11.rst > +++ b/doc/guides/rel_notes/release_22_11.rst > @@ -75,6 +75,11 @@ New Features > * Added ``rte_event_eth_tx_adapter_instance_get`` to get Tx adapter > instance ID for specified ethernet device ID and Tx queue index. > =20 > +* **Added uncore frequency control API to the power library.** > + > + Add api to allow uncore frequency adjustment. This is done through s/api/API/ > + manipulating related uncore frequency control sysfs entries to > + adjust the minimum and maximum uncore frequency values. It is Linux-only for Intel hardware only. > --- /dev/null > +++ b/lib/power/rte_power_uncore.c I would add "intel" in the filename. [...] > +#define UNCORE_FREQUENCY_DIR "/sys/devices/system/cpu/intel_uncore_frequ= ency" > +#define POWER_GOVERNOR_PERF "performance" > +#define POWER_UNCORE_SYSFILE_MAX_FREQ \ > + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02u_die_%02u/= max_freq_khz" > +#define POWER_UNCORE_SYSFILE_MIN_FREQ \ > + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02u_die_%02u/= min_freq_khz" > +#define POWER_UNCORE_SYSFILE_BASE_MAX_FREQ \ > + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02u_die_%02u/= initial_max_freq_khz" > +#define POWER_UNCORE_SYSFILE_BASE_MIN_FREQ \ > + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02u_die_%02u/= initial_min_freq_khz" It is for Intel CPU only, right? > + * This function should NOT be called in the fast path. > + * > + * @param pkg > + * Package number. > + * @param die > + * Die number. To me it is not clear what they are. Is it possible to better explain "pkg" and "die" somewhere? Is it related to NUMA nodes?