[dpdk-dev] [PATCH v5 07/10] net/i40e: implement power management API

[Guo, Jia]

> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov at intel.com>
> Sent: Wednesday, October 14, 2020 5:08 PM
> To: Guo, Jia <jia.guo at intel.com>; dev at dpdk.org
> Cc: Ma, Liang J <liang.j.ma at intel.com>; Xing, Beilei <beilei.xing at intel.com>;
> Hunt, David <david.hunt at intel.com>; Ananyev, Konstantin
> <konstantin.ananyev at intel.com>; jerinjacobk at gmail.com; Richardson,
> Bruce <bruce.richardson at intel.com>; thomas at monjalon.net; McDaniel,
> Timothy <timothy.mcdaniel at intel.com>; Eads, Gage
> <gage.eads at intel.com>; Macnamara, Chris <chris.macnamara at intel.com>
> Subject: Re: [PATCH v5 07/10] net/i40e: implement power management API
> 
> On 14-Oct-20 4:19 AM, Guo, Jia wrote:
> >
> >> -----Original Message-----
> >> From: Burakov, Anatoly <anatoly.burakov at intel.com>
> >> Sent: Saturday, October 10, 2020 12:02 AM
> >> To: dev at dpdk.org
> >> Cc: Ma, Liang J <liang.j.ma at intel.com>; Xing, Beilei
> >> <beilei.xing at intel.com>; Guo, Jia <jia.guo at intel.com>; Hunt, David
> >> <david.hunt at intel.com>; Ananyev, Konstantin
> >> <konstantin.ananyev at intel.com>; jerinjacobk at gmail.com; Richardson,
> >> Bruce <bruce.richardson at intel.com>; thomas at monjalon.net; McDaniel,
> >> Timothy <timothy.mcdaniel at intel.com>; Eads, Gage
> >> <gage.eads at intel.com>; Macnamara, Chris <chris.macnamara at intel.com>
> >> Subject: [PATCH v5 07/10] net/i40e: implement power management API
> >>
> >> From: Liang Ma <liang.j.ma at intel.com>
> >>
> >> Implement support for the power management API by implementing a
> >> `get_wake_addr` function that will return an address of an RX ring's status
> bit.
> >>
> >> Signed-off-by: Liang Ma <liang.j.ma at intel.com>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
> >> ---
> >>   drivers/net/i40e/i40e_ethdev.c |  1 +
> >>   drivers/net/i40e/i40e_rxtx.c   | 23 +++++++++++++++++++++++
> >>   drivers/net/i40e/i40e_rxtx.h   |  2 ++
> >>   3 files changed, 26 insertions(+)
> >>
> >> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >> b/drivers/net/i40e/i40e_ethdev.c index 943cfe71dc..cab86f8ec9 100644
> >> --- a/drivers/net/i40e/i40e_ethdev.c
> >> +++ b/drivers/net/i40e/i40e_ethdev.c
> >> @@ -513,6 +513,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops
> = {
> >>   	.mtu_set                      = i40e_dev_mtu_set,
> >>   	.tm_ops_get                   = i40e_tm_ops_get,
> >>   	.tx_done_cleanup              = i40e_tx_done_cleanup,
> >> +	.get_wake_addr	              = i40e_get_wake_addr,
> >>   };
> >>
> >>   /* store statistics names and its offset in stats structure */ diff
> >> --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> >> index 322fc1ed75..c17f27292f 100644
> >> --- a/drivers/net/i40e/i40e_rxtx.c
> >> +++ b/drivers/net/i40e/i40e_rxtx.c
> >> @@ -71,6 +71,29 @@
> >>   #define I40E_TX_OFFLOAD_NOTSUP_MASK \
> >>   		(PKT_TX_OFFLOAD_MASK ^ I40E_TX_OFFLOAD_MASK)
> >>
> >> +int
> >> +i40e_get_wake_addr(void *rx_queue, volatile void **tail_desc_addr,
> >> +		uint64_t *expected, uint64_t *mask) {
> >> +	struct i40e_rx_queue *rxq = rx_queue;
> >> +	volatile union i40e_rx_desc *rxdp;
> >> +	uint16_t desc;
> >> +
> >> +	desc = rxq->rx_tail;
> >> +	rxdp = &rxq->rx_ring[desc];
> >> +	/* watch for changes in status bit */
> >> +	*tail_desc_addr = &rxdp->wb.qword1.status_error_len;
> >> +
> >> +	/*
> >> +	 * we expect the DD bit to be set to 1 if this descriptor was already
> >> +	 * written to.
> >> +	 */
> >> +	*expected = rte_cpu_to_le_64(1 <<
> >> I40E_RX_DESC_STATUS_DD_SHIFT);
> >> +	*mask = rte_cpu_to_le_64(1 << I40E_RX_DESC_STATUS_DD_SHIFT);
> >> +
> >> +	return 0;
> >
> > Suppose that it will always success to get wake addr in i40e, right?
> 
> Yes. We've already checked all the parameters (queue etc.) in ethdev, so
> once we're here, that means there's no way this could fail as far as i can tell.
> 

Ok. 
Acked-by: Jeff Guo <jia.guo at intel.com>

> >
> >> +}
> >> +
> >>   static inline void
> >>   i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union
> >> i40e_rx_desc
> >> *rxdp)  { diff --git a/drivers/net/i40e/i40e_rxtx.h
> >> b/drivers/net/i40e/i40e_rxtx.h index 57d7b4160b..f23a2073e3 100644
> >> --- a/drivers/net/i40e/i40e_rxtx.h
> >> +++ b/drivers/net/i40e/i40e_rxtx.h
> >> @@ -248,6 +248,8 @@ uint16_t i40e_recv_scattered_pkts_vec_avx2(void
> >> *rx_queue,
> >>   	struct rte_mbuf **rx_pkts, uint16_t nb_pkts);  uint16_t
> >> i40e_xmit_pkts_vec_avx2(void *tx_queue, struct rte_mbuf **tx_pkts,
> >>   	uint16_t nb_pkts);
> >> +int i40e_get_wake_addr(void *rx_queue, volatile void **tail_desc_addr,
> >> +		uint64_t *expected, uint64_t *value);
> >>
> >>   /* For each value it means, datasheet of hardware can tell more details
> >>    *
> >> --
> >> 2.17.1
> 
> 
> --
> Thanks,
> Anatoly