On 14-Oct-20 4:10 AM, Guo, Jia wrote:
-----Original Message-----
From: dev <dev-boun...@dpdk.org> On Behalf Of Anatoly Burakov
Sent: Saturday, October 10, 2020 12:02 AM
To: dev@dpdk.org
Cc: Ma, Liang J <liang.j...@intel.com>; Thomas Monjalon
<tho...@monjalon.net>; Yigit, Ferruh <ferruh.yi...@intel.com>; Andrew
Rybchenko <andrew.rybche...@oktetlabs.ru>; Ray Kinsella
<m...@ashroe.eu>; Neil Horman <nhor...@tuxdriver.com>; Hunt, David
<david.h...@intel.com>; Ananyev, Konstantin
<konstantin.anan...@intel.com>; jerinjac...@gmail.com; Richardson,
Bruce <bruce.richard...@intel.com>; McDaniel, Timothy
<timothy.mcdan...@intel.com>; Eads, Gage <gage.e...@intel.com>;
Macnamara, Chris <chris.macnam...@intel.com>
Subject: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
management API
From: Liang Ma <liang.j...@intel.com>
Add a simple API to allow getting address of next RX descriptor from the
PMD, as well as release notes information.
Signed-off-by: Liang Ma <liang.j...@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
---
Hi Jia,
Thanks for your review. Responses below.
Notes:
v5:
- Bring function format in line with other functions in the file
- Ensure the API is supported by the driver before calling it (Konstantin)
doc/guides/rel_notes/release_20_11.rst | 16 ++++++++++++++
lib/librte_ethdev/rte_ethdev.c | 17 ++++++++++++++
lib/librte_ethdev/rte_ethdev.h | 24 ++++++++++++++++++++
lib/librte_ethdev/rte_ethdev_driver.h | 28 ++++++++++++++++++++++++
lib/librte_ethdev/rte_ethdev_version.map | 1 +
5 files changed, 86 insertions(+)
diff --git a/doc/guides/rel_notes/release_20_11.rst
b/doc/guides/rel_notes/release_20_11.rst
index 808bdc4e54..e85af5d3e9 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -55,6 +55,11 @@ New Features
Also, make sure to start the actual text at the margin.
=======================================================
+* **ethdev: add 1 new EXPERIMENTAL API for PMD power
management.**
+
+ * ``rte_eth_get_wake_addr()``
+ * add new eth_dev_ops ``get_wake_addr``
+
* **Updated Broadcom bnxt driver.**
Updated the Broadcom bnxt driver with new features and improvements,
including:
@@ -136,6 +141,17 @@ New Features
* Extern objects and functions can be plugged into the pipeline.
* Transaction-oriented table updates.
+* **Add PMD power management mechanism**
+
+ 3 new Ethernet PMD power management mechanism is added through
" mechanisms are " please.
+ existing RX callback infrastructure.
+
+ * Add power saving scheme based on UMWAIT instruction (x86 only)
+ * Add power saving scheme based on ``rte_pause()``
+ * Add power saving scheme based on frequency scaling through the
+ power library
+ * Add new EXPERIMENTAL API
``rte_power_pmd_mgmt_queue_enable()``
+ * Add new EXPERIMENTAL API
``rte_power_pmd_mgmt_queue_disable()``
+
Could this doc be separate to other specific patch if it is not related with
this patch?
It is related - it's the doc changes that add mention of this API. I was
under the impression current policy was having doc updates in the same
patch as the changes made?
Removed Items
-------------
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 48d1333b17..352108f43c 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4804,6 +4804,23 @@ rte_eth_tx_burst_mode_get(uint16_t port_id,
uint16_t queue_id,
dev->dev_ops->tx_burst_mode_get(dev, queue_id,
mode)); }
+int
+rte_eth_get_wake_addr(uint16_t port_id, uint16_t queue_id,
+ volatile void **wake_addr, uint64_t *expected, uint64_t
*mask) {
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_wake_addr, -
ENOTSUP);
+
+ return eth_err(port_id,
+ dev->dev_ops->get_wake_addr(dev->data-
rx_queues[queue_id],
+ wake_addr, expected, mask));
+}
+
int
rte_eth_dev_set_mc_addr_list(uint16_t port_id,
struct rte_ether_addr *mc_addr_set, diff --git
a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h index
d2bf74f128..a6cfe3cd57 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4014,6 +4014,30 @@ __rte_experimental int
rte_eth_tx_burst_mode_get(uint16_t port_id, uint16_t queue_id,
struct rte_eth_burst_mode *mode);
+/**
+ * Retrieve the wake up address from specific queue
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param queue_id
+ * The Tx queue on the Ethernet device for which information
+ * will be retrieved.
+ * @param wake_addr
+ * The pointer point to the address which is used for monitoring.
+ * @param expected
+ * The pointer point to value to be expected when descriptor is set.
+ * @param mask
+ * The pointer point to comparison bitmask for the expected value.
+ *
+ * @return
+ * - 0: Success.
+ * -EINVAL: Failed to get wake address.
+ */
Is that "-EINVAL " is the only error value which will be return?
Also -ENOTSUP, i'll add this, thanks.
+__rte_experimental
+int rte_eth_get_wake_addr(uint16_t port_id, uint16_t queue_id,
+ volatile void **wake_addr,
+ uint64_t *expected, uint64_t *mask);
+
/**
* Retrieve device registers and register attributes (number of registers and
* register size)
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h
b/lib/librte_ethdev/rte_ethdev_driver.h
index c3062c246c..935d46f25c 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -574,6 +574,31 @@ typedef int (*eth_tx_hairpin_queue_setup_t)
uint16_t nb_tx_desc,
const struct rte_eth_hairpin_conf *hairpin_conf);
+/**
+ * @internal
+ * Get the Wake up address.
+ *
+ * @param rxq
+ * Ethdev queue pointer.
+ * @param tail_desc_addr
+ * The pointer point to descriptor address var.
+ * @param expected
+ * The pointer point to value to be expected when descriptor is set.
+ * @param mask
+ * The pointer point to comparison bitmask for the expected value.
+ * @return
+ * Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ * Success.
+ * @retval -EINVAL
+ * Failed to get descriptor address.
+ */
The question is the same as above.
This is a driver function pointer, so return value will depend on driver
implementation. So far we only see 0 or -EINVAL values from the driver
itself, while -ENOTSUP will be returned by ethdev in case there is no
driver implementation of this function. So, in this case this is correct.
+typedef int (*eth_get_wake_addr_t)
+ (void *rxq, volatile void **tail_desc_addr,
+ uint64_t *expected, uint64_t *mask);
+
+
/**
* @internal A structure containing the functions exported by an Ethernet
driver.
*/
@@ -713,6 +738,9 @@ struct eth_dev_ops {
/**< Set up device RX hairpin queue. */
eth_tx_hairpin_queue_setup_t tx_hairpin_queue_setup;
/**< Set up device TX hairpin queue. */
+ eth_get_wake_addr_t get_wake_addr;
+ /**< Get wake up address. */
+
};
/**
diff --git a/lib/librte_ethdev/rte_ethdev_version.map
b/lib/librte_ethdev/rte_ethdev_version.map
index c95ef5157a..3cb2093980 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -229,6 +229,7 @@ EXPERIMENTAL {
# added in 20.11
rte_eth_link_speed_to_str;
rte_eth_link_to_str;
+ rte_eth_get_wake_addr;
};
INTERNAL {
--
2.17.1
--
Thanks,
Anatoly
