On 12-Oct-20 8:46 AM, Wang, Haiyue wrote:
Hi Liang,

-----Original Message-----
From: Burakov, Anatoly <[email protected]>
Sent: Saturday, October 10, 2020 00:02
To: [email protected]
Cc: Ma, Liang J <[email protected]>; Guo, Jia <[email protected]>; Wang, 
Haiyue
<[email protected]>; Hunt, David <[email protected]>; Ananyev, Konstantin
<[email protected]>; [email protected]; Richardson, Bruce 
<[email protected]>;
[email protected]; McDaniel, Timothy <[email protected]>; Eads, Gage 
<[email protected]>;
Macnamara, Chris <[email protected]>
Subject: [PATCH v5 06/10] net/ixgbe: implement power management API

From: Liang Ma <[email protected]>

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: Anatoly Burakov <[email protected]>
Signed-off-by: Liang Ma <[email protected]>
---
  drivers/net/ixgbe/ixgbe_ethdev.c |  1 +
  drivers/net/ixgbe/ixgbe_rxtx.c   | 22 ++++++++++++++++++++++
  drivers/net/ixgbe/ixgbe_rxtx.h   |  2 ++
  3 files changed, 25 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0b98e210e7..30b3f416d4 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -588,6 +588,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
  .udp_tunnel_port_del  = ixgbe_dev_udp_tunnel_port_del,
  .tm_ops_get           = ixgbe_tm_ops_get,
  .tx_done_cleanup      = ixgbe_dev_tx_done_cleanup,
+.get_wake_addr        = ixgbe_get_wake_addr,
  };

  /*
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 977ecf5137..7a9fd2aec6 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1366,6 +1366,28 @@ const uint32_t
  RTE_PTYPE_INNER_L3_IPV4_EXT | RTE_PTYPE_INNER_L4_UDP,
  };

+int ixgbe_get_wake_addr(void *rx_queue, volatile void **tail_desc_addr,
+uint64_t *expected, uint64_t *mask)
+{
+volatile union ixgbe_adv_rx_desc *rxdp;
+struct ixgbe_rx_queue *rxq = rx_queue;
+uint16_t desc;
+
+desc = rxq->rx_tail;
+rxdp = &rxq->rx_ring[desc];
+/* watch for changes in status bit */
+*tail_desc_addr = &rxdp->wb.upper.status_error;
+
+/*
+ * we expect the DD bit to be set to 1 if this descriptor was already
+ * written to.
+ */
+*expected = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
+*mask = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
+
+return 0;
+}
+

I'm wondering that whether the '.get_wake_addr' can be specific to
like 'rxq_tailq_addr_get' ? So that one day this wake up mechanism
can be applied to 'txq_tailq_addr_get' ? :-)

Also, "volatile void **tail_desc_addr, uint64_t *expected, uint64_t *mask"
can be merged into 'struct xxx' ? So that you can expand the API easily.

Just my thoughts.

Anyway, LGTM

Acked-by: Haiyue Wang <[email protected]>

Great point, will think of how to address it.


--
2.17.1


--
Thanks,
Anatoly

Reply via email to