[PATCH net-next 0/5] net: ipa: more multi-channel event ring work

[PATCH net-next 0/5] net: ipa: more multi-channel event ring work

[Alex Elder]

This series makes a little more progress toward supporting multiple
channels with a single event ring.  The first removes the assumption
that consecutive events are associated with the same RX channel.

The second derives the channel associated with an event from the
event itself, and the next does a small cleanup enabled by that.

The fourth causes updates to occur for every event processed (rather
once).  And the final patch does a little more rework to make TX
completion have more in common with RX completion.

					-Alex

Alex Elder (5):
  net: ipa: don't assume one channel per event ring
  net: ipa: don't pass channel when mapping transaction
  net: ipa: pass GSI pointer to gsi_evt_ring_rx_update()
  net: ipa: call gsi_evt_ring_rx_update() unconditionally
  net: ipa: move more code out of gsi_channel_update()

 drivers/net/ipa/gsi.c         | 59 +++++++++++++++++++----------------
 drivers/net/ipa/gsi_private.h |  3 --
 drivers/net/ipa/gsi_trans.c   | 28 +++++------------
 3 files changed, 40 insertions(+), 50 deletions(-)

-- 
2.34.1

[PATCH net-next 1/5] net: ipa: don't assume one channel per event ring

[Alex Elder]

In gsi_evt_ring_rx_update(), use gsi_event_trans() repeatedly
to find the transaction associated with an event, rather than
assuming consecutive events are associated with the same channel.
This removes the only caller of gsi_trans_pool_next(), so get rid
of it.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c         | 14 ++++++--------
 drivers/net/ipa/gsi_private.h |  3 ---
 drivers/net/ipa/gsi_trans.c   | 16 ----------------
 3 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index df8af1f00fc8b..0e9064c043adf 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1366,15 +1366,11 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
 {
 	struct gsi_channel *channel = evt_ring->channel;
 	struct gsi_ring *ring = &evt_ring->ring;
-	struct gsi_trans_info *trans_info;
 	struct gsi_event *event_done;
 	struct gsi_event *event;
-	struct gsi_trans *trans;
 	u32 event_avail;
 	u32 old_index;
 
-	trans_info = &channel->trans_info;
-
 	/* We'll start with the oldest un-processed event.  RX channels
 	 * replenish receive buffers in single-TRE transactions, so we
 	 * can just map that event to its transaction.  Transactions
@@ -1382,9 +1378,6 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
 	 */
 	old_index = ring->index;
 	event = gsi_ring_virt(ring, old_index);
-	trans = gsi_event_trans(channel->gsi, event);
-	if (!trans)
-		return;
 
 	/* Compute the number of events to process before we wrap,
 	 * and determine when we'll be done processing events.
@@ -1392,6 +1385,12 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
 	event_avail = ring->count - old_index % ring->count;
 	event_done = gsi_ring_virt(ring, index);
 	do {
+		struct gsi_trans *trans;
+
+		trans = gsi_event_trans(channel->gsi, event);
+		if (!trans)
+			return;
+
 		trans->len = __le16_to_cpu(event->len);
 
 		/* Move on to the next event and transaction */
@@ -1399,7 +1398,6 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
 			event++;
 		else
 			event = gsi_ring_virt(ring, 0);
-		trans = gsi_trans_pool_next(&trans_info->pool, trans);
 	} while (event != event_done);
 }
 
diff --git a/drivers/net/ipa/gsi_private.h b/drivers/net/ipa/gsi_private.h
index 74cbc287fc715..0b2516fa21b5d 100644
--- a/drivers/net/ipa/gsi_private.h
+++ b/drivers/net/ipa/gsi_private.h
@@ -16,9 +16,6 @@ struct gsi_channel;
 
 #define GSI_RING_ELEMENT_SIZE	16	/* bytes; must be a power of 2 */
 
-/* Return the entry that follows one provided in a transaction pool */
-void *gsi_trans_pool_next(struct gsi_trans_pool *pool, void *element);
-
 /**
  * gsi_trans_move_complete() - Mark a GSI transaction completed
  * @trans:	Transaction to commit
diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index a110be72f70b6..54a2400cb560e 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -214,22 +214,6 @@ void *gsi_trans_pool_alloc_dma(struct gsi_trans_pool *pool, dma_addr_t *addr)
 	return pool->base + offset;
 }
 
-/* Return the pool element that immediately follows the one given.
- * This only works done if elements are allocated one at a time.
- */
-void *gsi_trans_pool_next(struct gsi_trans_pool *pool, void *element)
-{
-	void *end = pool->base + pool->count * pool->size;
-
-	WARN_ON(element < pool->base);
-	WARN_ON(element >= end);
-	WARN_ON(pool->max_alloc != 1);
-
-	element += pool->size;
-
-	return element < end ? element : pool->base;
-}
-
 /* Map a given ring entry index to the transaction associated with it */
 static void gsi_channel_trans_map(struct gsi_channel *channel, u32 index,
 				  struct gsi_trans *trans)
-- 
2.34.1

[PATCH net-next 2/5] net: ipa: don't pass channel when mapping transaction

[Alex Elder]

Change gsi_channel_trans_map() so it derives the channel used from
the transaction.  Pass the index of the *first* TRE used by the
transaction, and have the called function account for the fact that
the last one used is what's important.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi_trans.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c
index 54a2400cb560e..cf646dc8e36a3 100644
--- a/drivers/net/ipa/gsi_trans.c
+++ b/drivers/net/ipa/gsi_trans.c
@@ -214,10 +214,14 @@ void *gsi_trans_pool_alloc_dma(struct gsi_trans_pool *pool, dma_addr_t *addr)
 	return pool->base + offset;
 }
 
-/* Map a given ring entry index to the transaction associated with it */
-static void gsi_channel_trans_map(struct gsi_channel *channel, u32 index,
-				  struct gsi_trans *trans)
+/* Map a TRE ring entry index to the transaction it is associated with */
+static void gsi_trans_map(struct gsi_trans *trans, u32 index)
 {
+	struct gsi_channel *channel = &trans->gsi->channel[trans->channel_id];
+
+	/* The completion event will indicate the last TRE used */
+	index += trans->used_count - 1;
+
 	/* Note: index *must* be used modulo the ring count here */
 	channel->trans_info.map[index % channel->tre_ring.count] = trans;
 }
@@ -568,15 +572,15 @@ static void __gsi_trans_commit(struct gsi_trans *trans, bool ring_db)
 		gsi_trans_tre_fill(dest_tre, addr, len, last_tre, bei, opcode);
 		dest_tre++;
 	}
+	/* Associate the TRE with the transaction */
+	gsi_trans_map(trans, tre_ring->index);
+
 	tre_ring->index += trans->used_count;
 
 	trans->len = byte_count;
 	if (channel->toward_ipa)
 		gsi_trans_tx_committed(trans);
 
-	/* Associate the last TRE with the transaction */
-	gsi_channel_trans_map(channel, tre_ring->index - 1, trans);
-
 	gsi_trans_move_pending(trans);
 
 	/* Ring doorbell if requested, or if all TREs are allocated */
-- 
2.34.1

[PATCH net-next 3/5] net: ipa: pass GSI pointer to gsi_evt_ring_rx_update()

[Alex Elder]

The only reason the event ring's channel pointer is needed in
gsi_evt_ring_rx_update() is so we can get at its GSI pointer.

We can pass the GSI pointer as an argument, along with the event
ring ID, and thereby avoid using the event ring channel pointer.
This is another step toward no longer assuming an event ring
services a single channel.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 0e9064c043adf..2c531ba1af2eb 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1345,8 +1345,9 @@ gsi_event_trans(struct gsi *gsi, struct gsi_event *event)
 
 /**
  * gsi_evt_ring_rx_update() - Record lengths of received data
- * @evt_ring:	Event ring associated with channel that received packets
- * @index:	Event index in ring reported by hardware
+ * @gsi:		GSI pointer
+ * @evt_ring_id:	Event ring ID
+ * @index:		Event index in ring reported by hardware
  *
  * Events for RX channels contain the actual number of bytes received into
  * the buffer.  Every event has a transaction associated with it, and here
@@ -1362,9 +1363,9 @@ gsi_event_trans(struct gsi *gsi, struct gsi_event *event)
  *
  * Note that @index always refers to an element *within* the event ring.
  */
-static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
+static void gsi_evt_ring_rx_update(struct gsi *gsi, u32 evt_ring_id, u32 index)
 {
-	struct gsi_channel *channel = evt_ring->channel;
+	struct gsi_evt_ring *evt_ring = &gsi->evt_ring[evt_ring_id];
 	struct gsi_ring *ring = &evt_ring->ring;
 	struct gsi_event *event_done;
 	struct gsi_event *event;
@@ -1387,7 +1388,7 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index)
 	do {
 		struct gsi_trans *trans;
 
-		trans = gsi_event_trans(channel->gsi, event);
+		trans = gsi_event_trans(gsi, event);
 		if (!trans)
 			return;
 
@@ -1500,7 +1501,7 @@ static struct gsi_trans *gsi_channel_update(struct gsi_channel *channel)
 	if (channel->toward_ipa)
 		gsi_trans_tx_completed(trans);
 	else
-		gsi_evt_ring_rx_update(evt_ring, index);
+		gsi_evt_ring_rx_update(gsi, evt_ring_id, index);
 
 	gsi_trans_move_complete(trans);
 
-- 
2.34.1

[PATCH net-next 4/5] net: ipa: call gsi_evt_ring_rx_update() unconditionally

[Alex Elder]

When an RX transaction completes, we update the trans->len field to
contain the actual number of bytes received.  This is done in a loop
in gsi_evt_ring_rx_update().

Change that function so it checks the data transfer direction
recorded in the transaction, and only updates trans->len for RX
transfers.

Then call it unconditionally.  This means events for TX endpoints
will run through the loop without otherwise doing anything, but
this will change shortly.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 2c531ba1af2eb..d08f3e73d51fc 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1392,7 +1392,8 @@ static void gsi_evt_ring_rx_update(struct gsi *gsi, u32 evt_ring_id, u32 index)
 		if (!trans)
 			return;
 
-		trans->len = __le16_to_cpu(event->len);
+		if (trans->direction == DMA_FROM_DEVICE)
+			trans->len = __le16_to_cpu(event->len);
 
 		/* Move on to the next event and transaction */
 		if (--event_avail)
@@ -1500,8 +1501,7 @@ static struct gsi_trans *gsi_channel_update(struct gsi_channel *channel)
 	 */
 	if (channel->toward_ipa)
 		gsi_trans_tx_completed(trans);
-	else
-		gsi_evt_ring_rx_update(gsi, evt_ring_id, index);
+	gsi_evt_ring_rx_update(gsi, evt_ring_id, index);
 
 	gsi_trans_move_complete(trans);
 
-- 
2.34.1

[PATCH net-next 5/5] net: ipa: move more code out of gsi_channel_update()

[Alex Elder]

Move the processing done for TX channels in gsi_channel_update()
into gsi_evt_ring_rx_update().  The called function is called for
both RX and TX channels, so rename it to be gsi_evt_ring_update().
As a result, this code no longer assumes events in an event ring are
associated with just one channel.

Because all events in a ring are handled in that function, we can
move the call to gsi_trans_move_complete() there, and can ring the
event ring doorbell there as well after all new events in the ring
have been processed.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index d08f3e73d51fc..4e46974a69ecd 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1344,7 +1344,7 @@ gsi_event_trans(struct gsi *gsi, struct gsi_event *event)
 }
 
 /**
- * gsi_evt_ring_rx_update() - Record lengths of received data
+ * gsi_evt_ring_update() - Update transaction state from hardware
  * @gsi:		GSI pointer
  * @evt_ring_id:	Event ring ID
  * @index:		Event index in ring reported by hardware
@@ -1353,6 +1353,10 @@ gsi_event_trans(struct gsi *gsi, struct gsi_event *event)
  * the buffer.  Every event has a transaction associated with it, and here
  * we update transactions to record their actual received lengths.
  *
+ * When an event for a TX channel arrives we use information in the
+ * transaction to report the number of requests and bytes have been
+ * transferred.
+ *
  * This function is called whenever we learn that the GSI hardware has filled
  * new events since the last time we checked.  The ring's index field tells
  * the first entry in need of processing.  The index provided is the
@@ -1363,7 +1367,7 @@ gsi_event_trans(struct gsi *gsi, struct gsi_event *event)
  *
  * Note that @index always refers to an element *within* the event ring.
  */
-static void gsi_evt_ring_rx_update(struct gsi *gsi, u32 evt_ring_id, u32 index)
+static void gsi_evt_ring_update(struct gsi *gsi, u32 evt_ring_id, u32 index)
 {
 	struct gsi_evt_ring *evt_ring = &gsi->evt_ring[evt_ring_id];
 	struct gsi_ring *ring = &evt_ring->ring;
@@ -1372,10 +1376,12 @@ static void gsi_evt_ring_rx_update(struct gsi *gsi, u32 evt_ring_id, u32 index)
 	u32 event_avail;
 	u32 old_index;
 
-	/* We'll start with the oldest un-processed event.  RX channels
-	 * replenish receive buffers in single-TRE transactions, so we
-	 * can just map that event to its transaction.  Transactions
-	 * associated with completion events are consecutive.
+	/* Starting with the oldest un-processed event, determine which
+	 * transaction (and which channel) is associated with the event.
+	 * For RX channels, update each completed transaction with the
+	 * number of bytes that were actually received.  For TX channels
+	 * associated with a network device, report to the network stack
+	 * the number of transfers and bytes this completion represents.
 	 */
 	old_index = ring->index;
 	event = gsi_ring_virt(ring, old_index);
@@ -1394,6 +1400,10 @@ static void gsi_evt_ring_rx_update(struct gsi *gsi, u32 evt_ring_id, u32 index)
 
 		if (trans->direction == DMA_FROM_DEVICE)
 			trans->len = __le16_to_cpu(event->len);
+		else
+			gsi_trans_tx_completed(trans);
+
+		gsi_trans_move_complete(trans);
 
 		/* Move on to the next event and transaction */
 		if (--event_avail)
@@ -1401,6 +1411,9 @@ static void gsi_evt_ring_rx_update(struct gsi *gsi, u32 evt_ring_id, u32 index)
 		else
 			event = gsi_ring_virt(ring, 0);
 	} while (event != event_done);
+
+	/* Tell the hardware we've handled these events */
+	gsi_evt_ring_doorbell(gsi, evt_ring_id, index);
 }
 
 /* Initialize a ring, including allocating DMA memory for its entries */
@@ -1499,14 +1512,7 @@ static struct gsi_trans *gsi_channel_update(struct gsi_channel *channel)
 	 * the number of transactions and bytes this completion represents
 	 * up the network stack.
 	 */
-	if (channel->toward_ipa)
-		gsi_trans_tx_completed(trans);
-	gsi_evt_ring_rx_update(gsi, evt_ring_id, index);
-
-	gsi_trans_move_complete(trans);
-
-	/* Tell the hardware we've handled these events */
-	gsi_evt_ring_doorbell(gsi, evt_ring_id, index);
+	gsi_evt_ring_update(gsi, evt_ring_id, index);
 
 	return gsi_channel_trans_complete(channel);
 }
-- 
2.34.1

Re: [PATCH net-next 0/5] net: ipa: more multi-channel event ring work

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 15 Jun 2022 11:59:24 -0500 you wrote:
> This series makes a little more progress toward supporting multiple
> channels with a single event ring.  The first removes the assumption
> that consecutive events are associated with the same RX channel.
> 
> The second derives the channel associated with an event from the
> event itself, and the next does a small cleanup enabled by that.
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] net: ipa: don't assume one channel per event ring
    https://git.kernel.org/netdev/net-next/c/dd5a046cbbed
  - [net-next,2/5] net: ipa: don't pass channel when mapping transaction
    https://git.kernel.org/netdev/net-next/c/8eec78319585
  - [net-next,3/5] net: ipa: pass GSI pointer to gsi_evt_ring_rx_update()
    https://git.kernel.org/netdev/net-next/c/2f48fb0edc0d
  - [net-next,4/5] net: ipa: call gsi_evt_ring_rx_update() unconditionally
    https://git.kernel.org/netdev/net-next/c/9f1c3ad65406
  - [net-next,5/5] net: ipa: move more code out of gsi_channel_update()
    https://git.kernel.org/netdev/net-next/c/81765eeac1b0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html