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 9BBBCA0A03; Mon, 18 Jan 2021 23:48:12 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 584D1140D2B; Mon, 18 Jan 2021 23:48:12 +0100 (CET) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mails.dpdk.org (Postfix) with ESMTP id 76863140D28 for ; Mon, 18 Jan 2021 23:48:10 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 941105C01C7; Mon, 18 Jan 2021 17:48:09 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Mon, 18 Jan 2021 17:48:09 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm3; bh= 6OZCvLY32skuGKSx2DOvC8m2tvcDq3mcVLCvz4pBREU=; b=DTJEaUacMAZN6f+y w90MBVm2YiZXeHmioz+BFV7peND11afeVS19Ssf2CMhDMgdOlKIFiHxhkc+5C/yY TCVnL3JEaRQJkYHRVDgeFqnpw+61SfcLV6D28m0PQyIGPTM9gdcGBvOpdFObivIf iPhlEW/sW/7KhswKlil1V6vGfQMGwQBTaVZLEd3ZcIDESo+WLg9h8e4i+zzZ8HKi dBDbwUoehSA4raJyRCaLzRdcW5vfN641AGJQoD+yCGiC6AEnWZ3lMi67YsrjAlX6 c+gbJKTJFt9mSHzVcVLZlEyxhPufmBOJwppc1D4ypm+CLfWasaBbWBgG1eDzjW0H U4QWjA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=6OZCvLY32skuGKSx2DOvC8m2tvcDq3mcVLCvz4pBR EU=; b=MP6W7I9Ql/iygZ3pmOi/DZBQ2GVQoPfqYV4SnHSeIs0zmTe6GJpJApAn1 YhvXWGKnjjTMMTJsE0TR1DUizvFTur2wmjd/6Gz4LmmVFqvJrO5MByKC57vQ7sVZ R0MPHSrlGXDH8jLb8jCOUX41eGnMP2nHIN0c1nyoQvWBsWUaxLh9ttnsKIjbOL9P HBELac7kCIHO5LweFvOtfdKo3YicI8c5GfnGSZxoh461uStT9JQdRu4n+7osz/tl J4OC4lHpYvwF8vNyoOiPuM6U81gmkr8vGHDN6MaKXzpHxu7Bvuc+SfptCyRgfrwo b6l3rA7EQhCx772Tgocbe6827tm3A== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrtdelucetufdoteggodetrfdotffvucfrrh hofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgenuceurghi lhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujfgurh ephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgrshcuofho nhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecuggftrfgrth htvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeuieeivdff geehnecukfhppeejjedrudefgedrvddtfedrudekgeenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhn vght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id E4A681080057; Mon, 18 Jan 2021 17:48:06 -0500 (EST) From: Thomas Monjalon To: Anatoly Burakov Cc: dev@dpdk.org, Liang Ma , David Hunt , Ray Kinsella , Neil Horman , konstantin.ananyev@intel.com, timothy.mcdaniel@intel.com, bruce.richardson@intel.com, chris.macnamara@intel.com Date: Mon, 18 Jan 2021 23:48:05 +0100 Message-ID: <6905373.rh0UmGlLeQ@thomas> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v17 07/11] power: add PMD power management API and callback 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 Sender: "dev" 14/01/2021 15:46, Anatoly Burakov: > From: Liang Ma > > + Currently, this power management API is limited to mandatory mapping of 1 > + queue to 1 core (multiple queues are supported, but they must be polled from > + different cores). This is quite limited. Not sure librte_power is the right place for a flexible ethdev management. > + > +API Overview for PMD Power Management > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Underlining should be shorter please. > +* **Queue Enable**: Enable specific power scheme for certain queue/port/core > + > +* **Queue Disable**: Disable power scheme for certain queue/port/core Please terminate sentences with a dot. > + > References > ---------- > > @@ -200,3 +241,6 @@ References > > * The :doc:`../sample_app_ug/vm_power_management` > chapter in the :doc:`../sample_app_ug/index` section. > + > +* The :doc:`../sample_app_ug/rxtx_callbacks` > + chapter in the :doc:`../sample_app_ug/index` section. Why the index page is mentionned here? > --- a/doc/guides/rel_notes/release_21_02.rst > +++ b/doc/guides/rel_notes/release_21_02.rst > +* **Add PMD power management helper API** Please follow release notes guidelines (past tense and dot). > + > + A new helper API has been added to make using Ethernet PMD power management > + easier for the user: ``rte_power_pmd_mgmt_queue_enable()``. Three power > + management schemes are supported initially: > + > + * Power saving based on UMWAIT instruction (x86 only) > + * Power saving based on ``rte_pause()`` (generic) or TPAUSE instruction (x86 only) > + * Power saving based on frequency scaling through the ``librte_power`` library [...] > --- a/lib/librte_power/meson.build > +++ b/lib/librte_power/meson.build > -deps += ['timer'] > +deps += ['timer' ,'ethdev'] Wrapping ethdev looks very strange to me. > --- /dev/null > +++ b/lib/librte_power/rte_power_pmd_mgmt.c > @@ -0,0 +1,364 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2010-2020 Intel Corporation 2010? > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "rte_power_pmd_mgmt.h" > + > +#define EMPTYPOLL_MAX 512 > + > +static struct pmd_conf_data { > + struct rte_cpu_intrinsics intrinsics_support; > + /**< what do we support? */ > + uint64_t tsc_per_us; > + /**< pre-calculated tsc diff for 1us */ > + uint64_t pause_per_us; > + /**< how many rte_pause can we fit in a microisecond? */ Vim typo spotted: microisecond > +} global_data; Not sure about the need for a struct. Please insert comment before the field if not on the same line. BTW, why doxygen syntax in a .c file? > --- /dev/null > +++ b/lib/librte_power/rte_power_pmd_mgmt.h > @@ -0,0 +1,90 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2010-2020 Intel Corporation > + */ > + > +#ifndef _RTE_POWER_PMD_MGMT_H > +#define _RTE_POWER_PMD_MGMT_H > + > +/** > + * @file > + * RTE PMD Power Management > + */ blank line here? > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/** > + * PMD Power Management Type > + */ > +enum rte_power_pmd_mgmt_type { > + /** Use power-optimized monitoring to wait for incoming traffic */ > + RTE_POWER_MGMT_TYPE_MONITOR = 1, > + /** Use power-optimized sleep to avoid busy polling */ > + RTE_POWER_MGMT_TYPE_PAUSE, > + /** Use frequency scaling when traffic is low */ > + RTE_POWER_MGMT_TYPE_SCALE, > +}; > + > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice Dot at the end please. > + * > + * Enable power management on a specified RX queue and lcore. > + * > + * @note This function is not thread-safe. > + * > + * @param lcore_id > + * lcore_id. Interesting comment :) > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The queue identifier of the Ethernet device. > + * @param mode > + * The power management callback function type. > + > + * @return > + * 0 on success > + * <0 on error > + */ > +__rte_experimental > +int > +rte_power_pmd_mgmt_queue_enable(unsigned int lcore_id, > + uint16_t port_id, uint16_t queue_id, > + enum rte_power_pmd_mgmt_type mode); In reality it is an API for ethdev Rx queue, not general PMD. The function should be renamed accordingly. [...] > --- a/lib/librte_power/version.map > +++ b/lib/librte_power/version.map > + # added in 21.02 > + rte_power_pmd_mgmt_queue_enable; > + rte_power_pmd_mgmt_queue_disable; Alpha sort please.