Bluetooth: controller: Support for separate ISO RX data path

Provides interface, data structures and demuxing capability for ISO RX
PDU allocation and transport from LLL to HCI.
Uses the RXFIFO composite for simplicity and reduced overhead.

Signed-off-by: Morten Priess <mtpr@oticon.com>

subsys/bluetooth/controller/Kconfig

@@ -198,6 +198,20 @@ config BT_CTLR_RX_BUFFERS
 	  connection interval and 2M PHY, maximum 18 packets with L2CAP payload
 	  size of 1 byte can be received.
 
+config BT_CTLR_ISO_RX_BUFFERS
+	int "Number of Isochronous Rx buffers"
+	depends on BT_CTLR_SYNC_ISO || BT_CTLR_CONN_ISO
+	default 8
+	range 1 30
+	help
+	  Set the number of Isochronous Rx PDUs to be buffered in the
+	  controller. Number of required RX buffers would worst-case be
+	  the number of RX nodes prepared in one ISO event for each
+	  active ISO group. This depends on the number of bursts in an
+	  ISO group and number of groups, and may need to be set lower
+	  that the theoretical maximum. Default of 8 is for supporting
+	  two groups of 4 payloads, e.g. 2 CIGs with 2 CISes of BN=2.
+
 config BT_CTLR_ISO_TX_BUFFERS
 	int "Number of Isochronous Tx buffers"
 	depends on BT_CTLR_ADV_ISO || BT_CTLR_CONN_ISO
@@ -216,6 +230,10 @@ config BT_CTLR_ISO_TX_BUFFER_SIZE
 	  Size of the Isochronous Tx buffers and the value returned in HCI LE
 	  Read Buffer Size V2 command response.
 
+config BT_CTLR_ISO_VENDOR_DATA_PATH
+	bool "Enable vendor-specific ISO data path"
+	depends on BT_CTLR_SYNC_ISO || BT_CTLR_CONN_ISO
+
 choice BT_CTLR_TX_PWR
 	prompt "Tx Power"
 	default BT_CTLR_TX_PWR_0

subsys/bluetooth/controller/hci/hci_driver.c

@@ -49,7 +49,6 @@
 #include "ll_sw/lll_sync_iso.h"
 #include "ll_sw/lll_conn.h"
 #include "ll_sw/lll_conn_iso.h"
-
 #include "ll_sw/isoal.h"
 
 #include "ll_sw/ull_iso_types.h"
@@ -229,9 +228,30 @@ static void prio_recv_thread(void *p1, void *p2, void *p3)
 	while (1) {
 		struct node_rx_pdu *node_rx;
 		struct net_buf *buf;
+		bool iso_received;
 		uint8_t num_cmplt;
 		uint16_t handle;
 
+		iso_received = false;
+
+#if defined(CONFIG_BT_CTLR_ISO)
+		node_rx = ll_iso_rx_get();
+		if (node_rx) {
+			ll_iso_rx_dequeue();
+
+			/* Find out and store the class for this node */
+			node_rx->hdr.user_meta = hci_get_class(node_rx);
+
+			/* Send the rx node up to Host thread,
+			 * recv_thread()
+			 */
+			BT_DBG("ISO RX node enqueue");
+			k_fifo_put(&recv_fifo, node_rx);
+
+			iso_received = true;
+		}
+#endif /* CONFIG_BT_CTLR_ISO */
+
 		/* While there are completed rx nodes */
 		while ((num_cmplt = ll_rx_get((void *)&node_rx, &handle))) {
 #if defined(CONFIG_BT_CONN)
@@ -280,13 +300,14 @@ static void prio_recv_thread(void *p1, void *p2, void *p3)
 				BT_DBG("RX node enqueue");
 				k_fifo_put(&recv_fifo, node_rx);
 			}
+		}
 
+		if (iso_received || node_rx) {
 			/* There may still be completed nodes, continue
 			 * pushing all those up to Host before waiting
 			 * for ULL mayfly
 			 */
 			continue;
-
 		}
 
 		BT_DBG("sem take...");
@@ -364,13 +385,24 @@ static inline struct net_buf *encode_node(struct node_rx_pdu *node_rx,
 		isoal_status_t err;
 
 		stream = ull_sync_iso_stream_get(node_rx->hdr.handle);
-		isoal_rx.meta = &node_rx->hdr.rx_iso_meta;
+
-		isoal_rx.pdu = (void *)node_rx->pdu;
+		/* Check validity of the data path sink. FIXME: A channel disconnect race
-		err = isoal_rx_pdu_recombine(stream->dp->sink_hdl, &isoal_rx);
+		 * may cause ISO data pending with without valid data path.
-		LL_ASSERT(err == ISOAL_STATUS_OK ||
+		 */
-			  err == ISOAL_STATUS_ERR_SDU_ALLOC);
+		if (stream && stream->dp && stream->dp->sink_hdl) {
+			isoal_rx.meta = &node_rx->hdr.rx_iso_meta;
+			isoal_rx.pdu = (void *)node_rx->pdu;
+			err = isoal_rx_pdu_recombine(stream->dp->sink_hdl, &isoal_rx);
+
+			LL_ASSERT(err == ISOAL_STATUS_OK ||
+				  err == ISOAL_STATUS_ERR_SDU_ALLOC);
+		}
 #endif /* CONFIG_BT_CTLR_SYNC_ISO */
-		break;
+
+		node_rx->hdr.next = NULL;
+		ll_iso_rx_mem_release((void **)&node_rx);
+
+		return buf;
 	}
 #endif /* CONFIG_BT_CTLR_ISO */
 

subsys/bluetooth/controller/include/ll.h

@@ -327,6 +327,7 @@ int ll_tx_mem_enqueue(uint16_t handle, void *node_tx);
 uint8_t ll_rx_get(void **node_rx, uint16_t *handle);
 void ll_rx_dequeue(void);
 void ll_rx_mem_release(void **node_rx);
+void ll_iso_rx_mem_release(void **node_rx);
 
 /* Downstream - ISO Data */
 void *ll_iso_tx_mem_acquire(void);

subsys/bluetooth/controller/ll_sw/lll.h

@@ -498,6 +498,15 @@ static inline void lll_hdr_init(void *lll, void *parent)
 #endif /* CONFIG_BT_CTLR_JIT_SCHEDULING */
 }
 
+/* If ISO vendor data path is not used, queue directly to ll_iso_rx */
+#if defined(CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH)
+#define iso_rx_put(link, rx) ull_iso_rx_put(link, rx)
+#define iso_rx_sched() ull_iso_rx_sched()
+#else
+#define iso_rx_put(link, rx) ll_iso_rx_put(link, rx)
+#define iso_rx_sched() ll_rx_sched()
+#endif /* CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH */
+
 void lll_done_score(void *param, uint8_t result);
 
 int lll_init(void);
@@ -529,12 +538,13 @@ void *ull_pdu_rx_alloc_peek(uint8_t count);
 void *ull_pdu_rx_alloc_peek_iter(uint8_t *idx);
 void *ull_pdu_rx_alloc(void);
 void *ull_iso_pdu_rx_alloc_peek(uint8_t count);
-void *ull_iso_pdu_rx_alloc_peek_iter(uint8_t *idx);
 void *ull_iso_pdu_rx_alloc(void);
 void ull_rx_put(memq_link_t *link, void *rx);
 void ull_rx_put_done(memq_link_t *link, void *done);
 void ull_rx_sched(void);
 void ull_rx_sched_done(void);
+void ull_iso_rx_put(memq_link_t *link, void *rx);
+void ull_iso_rx_sched(void);
 struct event_done_extra *ull_event_done_extra_get(void);
 struct event_done_extra *ull_done_extra_type_set(uint8_t type);
 void *ull_event_done(void *param);

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c

@@ -263,7 +263,7 @@ static int prepare_cb_common(struct lll_prepare_param *p)
 	radio_crc_configure(PDU_CRC_POLYNOMIAL, sys_get_le24(crc_init));
 	lll_chan_set(data_chan_use);
 
-	node_rx = ull_pdu_rx_alloc_peek(1U);
+	node_rx = ull_iso_pdu_rx_alloc_peek(1U);
 	LL_ASSERT(node_rx);
 	radio_pkt_rx_set(node_rx->pdu);
 
@@ -437,7 +437,7 @@ static void isr_rx(void *param)
 		    lll->ctrl) {
 			lll->cssn_curr = lll->cssn_next;
 
-			node_rx = ull_pdu_rx_alloc_peek(1U);
+			node_rx = ull_iso_pdu_rx_alloc_peek(1U);
 			LL_ASSERT(node_rx);
 
 			pdu = (void *)node_rx->pdu;
@@ -455,7 +455,7 @@ static void isr_rx(void *param)
 				}
 			}
 		} else {
-			node_rx = ull_pdu_rx_alloc_peek(3U);
+			node_rx = ull_iso_pdu_rx_alloc_peek(3U);
 			if (!node_rx) {
 				goto isr_rx_done;
 			}
@@ -480,7 +480,7 @@ static void isr_rx(void *param)
 		     (payload_index < lll->payload_head))) {
 			struct node_rx_iso_meta *iso_meta;
 
-			ull_pdu_rx_alloc();
+			ull_iso_pdu_rx_alloc();
 
 			node_rx->hdr.type = NODE_RX_TYPE_ISO_PDU;
 			node_rx->hdr.handle = lll->stream_handle[bis_idx];
@@ -601,7 +601,7 @@ isr_rx_find_subevent:
 				node_rx = lll->payload[bis_idx][payload_tail];
 				lll->payload[bis_idx][payload_tail] = NULL;
 
-				ull_rx_put(node_rx->hdr.link, node_rx);
+				iso_rx_put(node_rx->hdr.link, node_rx);
 			}
 
 			payload_index = payload_tail + 1U;
@@ -615,7 +615,7 @@ isr_rx_find_subevent:
 
 #if !defined(CONFIG_BT_CTLR_LOW_LAT_ULL)
 	if (node_rx) {
-		ull_rx_sched();
+		iso_rx_sched();
 	}
 #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL */
 
@@ -681,7 +681,7 @@ isr_rx_next_subevent:
 	}
 	lll_chan_set(data_chan_use);
 
-	node_rx = ull_pdu_rx_alloc_peek(1U);
+	node_rx = ull_iso_pdu_rx_alloc_peek(1U);
 	LL_ASSERT(node_rx);
 	radio_pkt_rx_set(node_rx->pdu);
 

subsys/bluetooth/controller/ll_sw/ull.c

@@ -1198,10 +1198,6 @@ void ll_rx_dequeue(void)
 	case NODE_RX_TYPE_CIS_ESTABLISHED:
 #endif /* CONFIG_BT_CTLR_CONN_ISO */
 
-#if defined(CONFIG_BT_CTLR_ISO)
-	case NODE_RX_TYPE_ISO_PDU:
-#endif
-
 #if defined(CONFIG_BT_CTLR_DF_SCAN_CTE_RX)
 	case NODE_RX_TYPE_SYNC_IQ_SAMPLE_REPORT:
 #endif /* CONFIG_BT_CTLR_DF_SCAN_CTE_RX */
@@ -2539,44 +2535,6 @@ static inline int rx_demux_rx(memq_link_t *link, struct node_rx_hdr *rx)
 	* CONFIG_BT_CONN
 	*/
 
-#if defined(CONFIG_BT_CTLR_ISO)
-	case NODE_RX_TYPE_ISO_PDU:
-	{
-		/* Remove from receive-queue; ULL has received this now */
-		(void)memq_dequeue(memq_ull_rx.tail, &memq_ull_rx.head, NULL);
-
-#if defined(CONFIG_BT_CTLR_CONN_ISO)
-		struct node_rx_pdu *rx_pdu = (struct node_rx_pdu *)rx;
-		struct ll_conn_iso_stream *cis =
-			ll_conn_iso_stream_get(rx_pdu->hdr.handle);
-		struct ll_iso_datapath *dp = cis->hdr.datapath_out;
-		isoal_sink_handle_t sink = dp->sink_hdl;
-
-		if (dp->path_id != BT_HCI_DATAPATH_ID_HCI) {
-			/* If vendor specific datapath pass to ISO AL here,
-			 * in case of HCI destination it will be passed in
-			 * HCI context.
-			 */
-			struct isoal_pdu_rx pckt_meta = {
-				.meta = &rx_pdu->hdr.rx_iso_meta,
-				.pdu  = (struct pdu_iso *) &rx_pdu->pdu[0]
-			};
-
-			/* Pass the ISO PDU through ISO-AL */
-			isoal_status_t err =
-				isoal_rx_pdu_recombine(sink, &pckt_meta);
-
-			LL_ASSERT(err == ISOAL_STATUS_OK); /* TODO handle err */
-		}
-#endif
-
-		/* Let ISO PDU start its long journey upwards */
-		ll_rx_put(link, rx);
-		ll_rx_sched();
-	}
-	break;
-#endif
-
 	default:
 	{
 #if defined(CONFIG_BT_CTLR_USER_EXT)

subsys/bluetooth/controller/ll_sw/ull_iso.c

@@ -14,6 +14,7 @@
 #include "util/memq.h"
 #include "util/mem.h"
 #include "util/mfifo.h"
+#include "util/mayfly.h"
 
 #include "pdu.h"
 
@@ -60,8 +61,32 @@ static int init_reset(void);
 #if BT_CTLR_ISO_STREAMS
 static struct ll_iso_datapath datapath_pool[BT_CTLR_ISO_STREAMS];
 #endif
+
 static void *datapath_free;
 
+#if defined(CONFIG_BT_CTLR_SYNC_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
+#define NODE_RX_HEADER_SIZE (offsetof(struct node_rx_pdu, pdu))
+/* ISO LL conformance tests require a PDU size of maximum 251 bytes + header */
+#define ISO_RX_BUFFER_SIZE (2 + 251)
+
+/* Declare the ISO rx node RXFIFO. This is a composite pool-backed MFIFO for
+ * rx_nodes. The declaration constructs the following data structures:
+ * - mfifo_iso_rx:    FIFO with pointers to PDU buffers
+ * - mem_iso_rx:      Backing data pool for PDU buffer elements
+ * - mem_link_iso_rx: Pool of memq_link_t elements
+ *
+ * Two extra links are reserved for use by the ll_iso_rx and ull_iso_rx memq.
+ */
+static RXFIFO_DEFINE(iso_rx, NODE_RX_HEADER_SIZE + ISO_RX_BUFFER_SIZE,
+			     CONFIG_BT_CTLR_ISO_RX_BUFFERS, 2);
+
+static MEMQ_DECLARE(ll_iso_rx);
+#if defined(CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH)
+static MEMQ_DECLARE(ull_iso_rx);
+static void iso_rx_demux(void *param);
+#endif /* CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH */
+#endif /* CONFIG_BT_CTLR_SYNC_ISO) || CONFIG_BT_CTLR_CONN_ISO */
+
 #if defined(CONFIG_BT_CTLR_ADV_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
 static MFIFO_DEFINE(iso_tx, sizeof(struct lll_tx),
 		    CONFIG_BT_CTLR_ISO_TX_BUFFERS);
@@ -522,6 +547,182 @@ int ull_iso_reset(void)
 	return 0;
 }
 
+#if defined(CONFIG_BT_CTLR_SYNC_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
+void *ull_iso_pdu_rx_alloc_peek(uint8_t count)
+{
+	if (count > MFIFO_AVAIL_COUNT_GET(iso_rx)) {
+		return NULL;
+	}
+
+	return MFIFO_DEQUEUE_PEEK(iso_rx);
+}
+
+void *ull_iso_pdu_rx_alloc(void)
+{
+	return MFIFO_DEQUEUE(iso_rx);
+}
+
+#if defined(CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH)
+void ull_iso_rx_put(memq_link_t *link, void *rx)
+{
+	/* Enqueue the Rx object */
+	memq_enqueue(link, rx, &memq_ull_iso_rx.tail);
+}
+
+void ull_iso_rx_sched(void)
+{
+	static memq_link_t link;
+	static struct mayfly mfy = {0, 0, &link, NULL, iso_rx_demux};
+
+	/* Kick the ULL (using the mayfly, tailchain it) */
+	mayfly_enqueue(TICKER_USER_ID_LLL, TICKER_USER_ID_ULL_HIGH, 1, &mfy);
+}
+
+static void iso_rx_demux(void *param)
+{
+	struct ll_conn_iso_stream *cis;
+	struct ll_iso_datapath *dp;
+	struct node_rx_pdu *rx_pdu;
+	isoal_sink_handle_t sink;
+	struct node_rx_hdr *rx;
+	memq_link_t *link;
+
+	do {
+		link = memq_peek(memq_ull_iso_rx.head, memq_ull_iso_rx.tail,
+				 (void **)&rx);
+		if (link) {
+			/* Demux Rx objects */
+			switch (rx->type) {
+			case NODE_RX_TYPE_RELEASE:
+				(void)memq_dequeue(memq_ull_iso_rx.tail,
+						   &memq_ull_iso_rx.head, NULL);
+				ll_iso_rx_put(link, rx);
+				ll_rx_sched();
+				break;
+
+			case NODE_RX_TYPE_ISO_PDU:
+				/* Remove from receive-queue; ULL has received this now */
+				(void)memq_dequeue(memq_ull_iso_rx.tail, &memq_ull_iso_rx.head,
+						   NULL);
+
+#if defined(CONFIG_BT_CTLR_CONN_ISO)
+				rx_pdu = (struct node_rx_pdu *)rx;
+				cis = ll_conn_iso_stream_get(rx_pdu->hdr.handle);
+				dp = cis->hdr.datapath_out;
+				sink = dp->sink_hdl;
+
+				if (dp->path_id != BT_HCI_DATAPATH_ID_HCI) {
+					/* If vendor specific datapath pass to ISO AL here,
+					 * in case of HCI destination it will be passed in
+					 * HCI context.
+					 */
+					struct isoal_pdu_rx pckt_meta = {
+						.meta = &rx_pdu->hdr.rx_iso_meta,
+						.pdu  = (struct pdu_iso *)&rx_pdu->pdu[0]
+					};
+
+					/* Pass the ISO PDU through ISO-AL */
+					const isoal_status_t err =
+						isoal_rx_pdu_recombine(sink, &pckt_meta);
+
+					LL_ASSERT(err == ISOAL_STATUS_OK); /* TODO handle err */
+				}
+#endif /* CONFIG_BT_CTLR_CONN_ISO */
+
+				/* Let ISO PDU start its long journey upwards */
+				ll_iso_rx_put(link, rx);
+				ll_rx_sched();
+				break;
+
+			default:
+				LL_ASSERT(0);
+				break;
+			}
+		}
+	} while (link);
+}
+#endif /* CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH */
+
+void ll_iso_rx_put(memq_link_t *link, void *rx)
+{
+	/* Enqueue the Rx object */
+	memq_enqueue(link, rx, &memq_ll_iso_rx.tail);
+}
+
+void *ll_iso_rx_get(void)
+{
+	struct node_rx_hdr *rx;
+	memq_link_t *link;
+
+	link = memq_peek(memq_ll_iso_rx.head, memq_ll_iso_rx.tail, (void **)&rx);
+	while (link) {
+		/* Do not send up buffers to Host thread that are
+		 * marked for release
+		 */
+		if (rx->type == NODE_RX_TYPE_RELEASE) {
+			(void)memq_dequeue(memq_ll_iso_rx.tail,
+						&memq_ll_iso_rx.head, NULL);
+			mem_release(link, &mem_link_iso_rx.free);
+			mem_release(rx, &mem_iso_rx.free);
+			RXFIFO_ALLOC(iso_rx, 1);
+
+			link = memq_peek(memq_ll_iso_rx.head, memq_ll_iso_rx.tail, (void **)&rx);
+			continue;
+		}
+		return rx;
+	}
+
+	return NULL;
+}
+
+void ll_iso_rx_dequeue(void)
+{
+	struct node_rx_hdr *rx = NULL;
+	memq_link_t *link;
+
+	link = memq_dequeue(memq_ll_iso_rx.tail, &memq_ll_iso_rx.head,
+			    (void **)&rx);
+	LL_ASSERT(link);
+
+	mem_release(link, &mem_link_iso_rx.free);
+
+	/* Handle object specific clean up */
+	switch (rx->type) {
+	case NODE_RX_TYPE_ISO_PDU:
+		break;
+	default:
+		LL_ASSERT(0);
+		break;
+	}
+}
+
+void ll_iso_rx_mem_release(void **node_rx)
+{
+	struct node_rx_hdr *rx;
+
+	rx = *node_rx;
+	while (rx) {
+		struct node_rx_hdr *rx_free;
+
+		rx_free = rx;
+		rx = rx->next;
+
+		switch (rx_free->type) {
+		case NODE_RX_TYPE_ISO_PDU:
+			mem_release(rx_free, &mem_iso_rx.free);
+			break;
+		default:
+			LL_ASSERT(0);
+			break;
+		}
+	}
+
+	*node_rx = rx;
+
+	RXFIFO_ALLOC(iso_rx, UINT8_MAX);
+}
+#endif /* CONFIG_BT_CTLR_SYNC_ISO) || CONFIG_BT_CTLR_CONN_ISO */
+
 void ull_iso_datapath_release(struct ll_iso_datapath *dp)
 {
 	mem_release(dp, &datapath_free);
@@ -529,6 +730,30 @@ void ull_iso_datapath_release(struct ll_iso_datapath *dp)
 
 static int init_reset(void)
 {
+#if defined(CONFIG_BT_CTLR_SYNC_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
+	memq_link_t *link;
+
+	RXFIFO_INIT(iso_rx);
+
+	/* Acquire a link to initialize ull rx memq */
+	link = mem_acquire(&mem_link_iso_rx.free);
+	LL_ASSERT(link);
+
+#if defined(CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH)
+	/* Initialize ull rx memq */
+	MEMQ_INIT(ull_iso_rx, link);
+#endif /* CONFIG_BT_CTLR_ISO_VENDOR_DATA_PATH */
+
+	/* Acquire a link to initialize ll_iso_rx memq */
+	link = mem_acquire(&mem_link_iso_rx.free);
+	LL_ASSERT(link);
+
+	/* Initialize ll_iso_rx memq */
+	MEMQ_INIT(ll_iso_rx, link);
+
+	RXFIFO_ALLOC(iso_rx, UINT8_MAX);
+#endif /* CONFIG_BT_CTLR_SYNC_ISO) || CONFIG_BT_CTLR_CONN_ISO */
+
 #if defined(CONFIG_BT_CTLR_ADV_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO)
 	/* Initialize tx pool. */
 	mem_init(mem_iso_tx.pool, CONFIG_BT_CTLR_ISO_TX_BUFFER_SIZE,

subsys/bluetooth/controller/ll_sw/ull_iso_internal.h

@@ -7,3 +7,6 @@
 int ull_iso_init(void);
 int ull_iso_reset(void);
 void ull_iso_datapath_release(struct ll_iso_datapath *dp);
+void ll_iso_rx_put(memq_link_t *link, void *rx);
+void *ll_iso_rx_get(void);
+void ll_iso_rx_dequeue(void);

tests/bluetooth/bsim_bt/bsim_test_iso/src/main.c

@@ -646,7 +646,7 @@ static void test_iso_recv_main(void)
 	uint8_t check_countdown = 3;
 
 	printk("Waiting for remote BIG terminate by checking for missing "
-	       "%u BIG Info report...", check_countdown);
+	       "%u BIG Info report...\n", check_countdown);
 	do {
 		is_sync_recv = false;
 		is_big_info = false;