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 82238A0544; Mon, 10 Oct 2022 14:46:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3E9B54021E; Mon, 10 Oct 2022 14:46:23 +0200 (CEST) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mails.dpdk.org (Postfix) with ESMTP id 5E1CF40146 for ; Mon, 10 Oct 2022 14:46:22 +0200 (CEST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 0BEE45C00C7; Mon, 10 Oct 2022 08:46:20 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Mon, 10 Oct 2022 08:46:20 -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=fm3; t=1665405980; x= 1665492380; bh=Eul7m4kubspgpFR/QB5S9/NMLzQm+COx2rfIIfIox10=; b=m MXS6Y6bwaKuEg4LFxNEZIMmGqzZwdXpDQ0OmZBu428b6cyKwb1SHmS0NXUJWTHLy JhMb0RLsHnpIQ1/DuD+Yxo5PkuRRWy+FqztPx6qsRK9eagbMH1e15fM/rTCqq3dM AY2TM6RTYJr+JlNmDvN/RCfcQ9DJFwvky3GGGJhfnRS5IvfYyBVOiVj2bxLzju9/ qzFu4lGqb2+0Yvq9wY01Od4+udoR47uXmP1Wzhlr22gJiB76iFiOxtJtx5aq/lG9 w65/n1FFcxGxB6Fwb0B0iiNpQrg6VHWMSkTYtSHle7/uzxyTGpujBY8hZTn/9jFG 1AQHwG0kMYYjeKH9Q3DEg== 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=fm3; t=1665405980; x= 1665492380; bh=Eul7m4kubspgpFR/QB5S9/NMLzQm+COx2rfIIfIox10=; b=o vwAS/bEvMc3wJ/wl4LRSZe2jhlhm7I0gsIPNDcaCoWBoyMq74aImRH2Muk5YpKRX oSgEQOTRehlFnaW7QYWXTulyODoLQzwX+oHnYPqcevS1LfaoHyzOCOjSQhDIQDwd b41nDZ49wXUkgLd4qALJWy7Uh8vS0C05rEu3mRlHWCUq7T8+nsNZ42Vyi7pIdCmp za5l+QHV6bG60tzFkVtnMlJYJSmutwUx/NFLsKxwHSE1jVOLVkNnfQrGS0ixNf0l Kj7QCwy+ZTYCG9Mi9G0UCdG6sn4S3jBXL3Ow8NAOcFuNDpvYnKNMSrzGizJWpNS8 KW0OI9EIFN7bl5sRbsLIQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeejgedghedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedtjeeiieefhedtfffgvdelteeufeefheeujefgueetfedttdei kefgkeduhedtgfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 10 Oct 2022 08:46:19 -0400 (EDT) From: Thomas Monjalon To: Tadhg Kearney Cc: dev@dpdk.org, david.hunt@intel.com, anatoly.burakov@intel.com, reshma.pattan@intel.com, david.marchand@redhat.com Subject: Re: [PATCH v9 1/3] power: add Intel uncore frequency control API to power library Date: Mon, 10 Oct 2022 14:46:17 +0200 Message-ID: <10475720.aFP6jjVeTY@thomas> In-Reply-To: <20221006093803.2076768-2-tadhg.kearney@intel.com> References: <20221005162023.1923558-1-tadhg.kearney@intel.com> <20221006093803.2076768-1-tadhg.kearney@intel.com> <20221006093803.2076768-2-tadhg.kearney@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 06/10/2022 11:38, Tadhg Kearney: > +API Overview for Intel Uncore > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +Overview of each function in the Intel Uncore API, with explanation of what > +they do. Each function should not be called in the fast path. > + > +* **Uncore Power Init** > + Initialize uncore power, populate frequency array and record > + original min & max for die on pkg. > + > +* **Uncore Power Exit** > + Exit uncore power, restoring original min & max for die on pkg. > + > +* **Get Uncore Power Freq** > + Get current uncore freq index for die on pkg. > + > +* **Set Uncore Power Freq** > + Set min & max uncore freq index for die on pkg to specified index value > + (min and max will be the same). > + > +* **Uncore Power Max** > + Set min & max uncore freq to maximum frequency index for die on pkg > + (min and max will be the same). > + > +* **Uncore Power Min** > + Set min & max uncore freq to minimum frequency index for die on pkg > + (min and max will be the same). > + > +* **Get Num Freqs** > + Get the number of frequencies in the index array. > + > +* **Get Num Pkgs** > + Get the number of packages (CPU's) on the system. > + > +* **Get Num Dies** > + Get the number of die's on a given package. This is the role of doxygen documentation to explain API. I don't understand why it is there. Anyway I've converted it into a definition list, which the proper RST syntax for what you do. > 'rte_power.c', > 'rte_power_empty_poll.c', > 'rte_power_pmd_mgmt.c', > + 'rte_power_intel_uncore.c', In general, we keep such list in alphabetical order. [...] > +struct uncore_power_info { > + unsigned int die; /** Core die id */ > + unsigned int pkg; /** Package id */ > + uint32_t freqs[MAX_UNCORE_FREQS];/** Frequency array */ > + uint32_t nb_freqs; /** Number of available freqs */ > + FILE *f_cur_min; /** FD of scaling_min */ > + FILE *f_cur_max; /** FD of scaling_max */ > + uint32_t curr_idx; /** Freq index in freqs array */ > + uint32_t org_min_freq; /** Original min freq of uncore */ > + uint32_t org_max_freq; /** Original max freq of uncore */ > + uint32_t init_max_freq; /** System max uncore freq */ > + uint32_t init_min_freq; /** System min uncore freq */ > +} __rte_cache_aligned; No need of doxygen syntax in a .c file. And an alignment is wrong. [...] > + RTE_LOG(DEBUG, POWER, "Invalid uncore frequency index %u, which " > + "should be less than %u\n", idx, ui->nb_freqs); When you have time, it would be good to switch to dynamic logging. See RTE_LOG_REGISTER_DEFAULT [...] > +#ifndef _RTE_POWER_INTEL_UNCORE_H > +#define _RTE_POWER_INTEL_UNCORE_H underscore prefix is reserved for reserved keywords [...] > +/** > + * Exit uncore frequency management on a specific die on a package. It will restore uncore min and Screen width is limited, while scrolling bar is infinite, so don't hesitate to break your lines shorter when possible, like at the end of a sentence.