[v5,04/10] ethdev: add simple power management API

Message ID 931cbea6d091f16a51ad7eed736b4b6e69df93aa.1602258833.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v5,01/10] eal: add new x86 cpuid support for WAITPKG |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Anatoly Burakov Oct. 9, 2020, 4:02 p.m. UTC
  From: Liang Ma <liang.j.ma@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.ma@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

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(+)
  

Comments

Guo, Jia Oct. 14, 2020, 3:10 a.m. UTC | #1
> -----Original Message-----
> From: dev <dev-bounces@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.ma@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
> <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Hunt, David
> <david.hunt@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
> Bruce <bruce.richardson@intel.com>; McDaniel, Timothy
> <timothy.mcdaniel@intel.com>; Eads, Gage <gage.eads@intel.com>;
> Macnamara, Chris <chris.macnamara@intel.com>
> Subject: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
> management API
> 
> From: Liang Ma <liang.j.ma@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.ma@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> 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?

> 
>  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?

> +__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.

> +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
  
Anatoly Burakov Oct. 14, 2020, 9:07 a.m. UTC | #2
On 14-Oct-20 4:10 AM, Guo, Jia wrote:
> 
>> -----Original Message-----
>> From: dev <dev-bounces@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.ma@intel.com>; Thomas Monjalon
>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
>> <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Hunt, David
>> <david.hunt@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
>> Bruce <bruce.richardson@intel.com>; McDaniel, Timothy
>> <timothy.mcdaniel@intel.com>; Eads, Gage <gage.eads@intel.com>;
>> Macnamara, Chris <chris.macnamara@intel.com>
>> Subject: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
>> management API
>>
>> From: Liang Ma <liang.j.ma@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.ma@intel.com>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@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
  
Guo, Jia Oct. 14, 2020, 9:15 a.m. UTC | #3
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Wednesday, October 14, 2020 5:07 PM
> To: Guo, Jia <jia.guo@intel.com>; dev@dpdk.org
> Cc: Ma, Liang J <liang.j.ma@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
> <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Hunt, David
> <david.hunt@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
> Bruce <bruce.richardson@intel.com>; McDaniel, Timothy
> <timothy.mcdaniel@intel.com>; Eads, Gage <gage.eads@intel.com>;
> Macnamara, Chris <chris.macnamara@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
> management API
> 
> On 14-Oct-20 4:10 AM, Guo, Jia wrote:
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@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.ma@intel.com>; Thomas Monjalon
> >> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> >> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
> >> <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Hunt, David
> >> <david.hunt@intel.com>; Ananyev, Konstantin
> >> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
> >> Bruce <bruce.richardson@intel.com>; McDaniel, Timothy
> >> <timothy.mcdaniel@intel.com>; Eads, Gage <gage.eads@intel.com>;
> >> Macnamara, Chris <chris.macnamara@intel.com>
> >> Subject: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
> >> management API
> >>
> >> From: Liang Ma <liang.j.ma@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.ma@intel.com>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@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?
> 

Do you think this part would be better separate into [PATCH v5 05/10]?

> >
> >>
> >>   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.
> 

Ok.

> >
> >> +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
  
Bruce Richardson Oct. 14, 2020, 9:23 a.m. UTC | #4
On Wed, Oct 14, 2020 at 10:07:09AM +0100, Burakov, Anatoly wrote:
> On 14-Oct-20 4:10 AM, Guo, Jia wrote:
> > 
> > > -----Original Message-----
> > > From: dev <dev-bounces@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.ma@intel.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> > > Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
> > > <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Hunt, David
> > > <david.hunt@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
> > > Bruce <bruce.richardson@intel.com>; McDaniel, Timothy
> > > <timothy.mcdaniel@intel.com>; Eads, Gage <gage.eads@intel.com>;
> > > Macnamara, Chris <chris.macnamara@intel.com>
> > > Subject: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
> > > management API
> > > 
> > > From: Liang Ma <liang.j.ma@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.ma@intel.com>
> > > Signed-off-by: Anatoly Burakov <anatoly.burakov@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?
> 

Yes, that is the case. Doc changes should be made alongside the relevant
code changes.
  
Anatoly Burakov Oct. 14, 2020, 9:30 a.m. UTC | #5
On 14-Oct-20 10:15 AM, Guo, Jia wrote:
> 
>> -----Original Message-----
>> From: Burakov, Anatoly <anatoly.burakov@intel.com>
>> Sent: Wednesday, October 14, 2020 5:07 PM
>> To: Guo, Jia <jia.guo@intel.com>; dev@dpdk.org
>> Cc: Ma, Liang J <liang.j.ma@intel.com>; Thomas Monjalon
>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
>> <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Hunt, David
>> <david.hunt@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
>> Bruce <bruce.richardson@intel.com>; McDaniel, Timothy
>> <timothy.mcdaniel@intel.com>; Eads, Gage <gage.eads@intel.com>;
>> Macnamara, Chris <chris.macnamara@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
>> management API
>>
>> On 14-Oct-20 4:10 AM, Guo, Jia wrote:
>>>
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@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.ma@intel.com>; Thomas Monjalon
>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>>>> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella
>>>> <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Hunt, David
>>>> <david.hunt@intel.com>; Ananyev, Konstantin
>>>> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
>>>> Bruce <bruce.richardson@intel.com>; McDaniel, Timothy
>>>> <timothy.mcdaniel@intel.com>; Eads, Gage <gage.eads@intel.com>;
>>>> Macnamara, Chris <chris.macnamara@intel.com>
>>>> Subject: [dpdk-dev] [PATCH v5 04/10] ethdev: add simple power
>>>> management API
>>>>
>>>> From: Liang Ma <liang.j.ma@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.ma@intel.com>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@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?
>>
> 
> Do you think this part would be better separate into [PATCH v5 05/10]?

Oh, sorry, you're right, these aren't related, as this functionality 
isn't in this patch yet. Will fix.

> 
>>>
>>>>
>>>>    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.
>>
> 
> Ok.
> 
>>>
>>>> +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
  

Patch

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 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()``
+
 
 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.
+ */
+__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.
+ */
+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 {