diff mbox series

[v5,07/10] net/i40e: implement power management API

Message ID 78bfa354463be2c3560ee97c369ae7266e0fb50f.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>

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@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@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(+)

Comments

Guo, Jia Oct. 14, 2020, 3:19 a.m. UTC | #1 
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Saturday, October 10, 2020 12:02 AM
> To: dev@dpdk.org
> Cc: Ma, Liang J <liang.j.ma@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Guo, Jia <jia.guo@intel.com>; Hunt, David <david.hunt@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> jerinjacobk@gmail.com; Richardson, Bruce <bruce.richardson@intel.com>;
> thomas@monjalon.net; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
> Eads, Gage <gage.eads@intel.com>; Macnamara, Chris
> <chris.macnamara@intel.com>
> Subject: [PATCH v5 07/10] net/i40e: implement power management API
> 
> From: Liang Ma <liang.j.ma@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@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@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?

> +}
> +
>  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
Anatoly Burakov Oct. 14, 2020, 9:08 a.m. UTC | #2 
On 14-Oct-20 4:19 AM, Guo, Jia wrote:
> 
>> -----Original Message-----
>> From: Burakov, Anatoly <anatoly.burakov@intel.com>
>> Sent: Saturday, October 10, 2020 12:02 AM
>> To: dev@dpdk.org
>> Cc: Ma, Liang J <liang.j.ma@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
>> Guo, Jia <jia.guo@intel.com>; Hunt, David <david.hunt@intel.com>;
>> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
>> jerinjacobk@gmail.com; Richardson, Bruce <bruce.richardson@intel.com>;
>> thomas@monjalon.net; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
>> Eads, Gage <gage.eads@intel.com>; Macnamara, Chris
>> <chris.macnamara@intel.com>
>> Subject: [PATCH v5 07/10] net/i40e: implement power management API
>>
>> From: Liang Ma <liang.j.ma@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@intel.com>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@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.

> 
>> +}
>> +
>>   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
Guo, Jia Oct. 14, 2020, 9:17 a.m. UTC | #3 
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Wednesday, October 14, 2020 5:08 PM
> To: Guo, Jia <jia.guo@intel.com>; dev@dpdk.org
> Cc: Ma, Liang J <liang.j.ma@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Hunt, David <david.hunt@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
> Bruce <bruce.richardson@intel.com>; thomas@monjalon.net; McDaniel,
> Timothy <timothy.mcdaniel@intel.com>; Eads, Gage
> <gage.eads@intel.com>; Macnamara, Chris <chris.macnamara@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@intel.com>
> >> Sent: Saturday, October 10, 2020 12:02 AM
> >> To: dev@dpdk.org
> >> Cc: Ma, Liang J <liang.j.ma@intel.com>; Xing, Beilei
> >> <beilei.xing@intel.com>; Guo, Jia <jia.guo@intel.com>; Hunt, David
> >> <david.hunt@intel.com>; Ananyev, Konstantin
> >> <konstantin.ananyev@intel.com>; jerinjacobk@gmail.com; Richardson,
> >> Bruce <bruce.richardson@intel.com>; thomas@monjalon.net; McDaniel,
> >> Timothy <timothy.mcdaniel@intel.com>; Eads, Gage
> >> <gage.eads@intel.com>; Macnamara, Chris <chris.macnamara@intel.com>
> >> Subject: [PATCH v5 07/10] net/i40e: implement power management API
> >>
> >> From: Liang Ma <liang.j.ma@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@intel.com>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@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@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
diff mbox series

Patch

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;
+}
+
 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
  *