Bluetooth: Controller: Replace prepare pipeline with ordered list

Replace prepare pipeline with ordered linked list.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

Author: cvinayak

subsys/bluetooth/controller/ll_sw/lll.h

@@ -611,7 +611,7 @@ struct lll_event *ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
 				      lll_prepare_cb_t prepare_cb,
 				      uint8_t is_resume);
 void *ull_prepare_dequeue_get(void);
-void *ull_prepare_dequeue_iter(uint8_t *idx);
+void *ull_prepare_dequeue_iter(void **idx);
 void ull_prepare_dequeue(uint8_t caller_id);
 void *ull_pdu_rx_alloc_peek(uint8_t count);
 void *ull_pdu_rx_alloc_peek_iter(uint8_t *idx);

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

@@ -71,7 +71,7 @@ static int init_reset(void);
 static inline void done_inc(void);
 #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL_DONE */
 static inline bool is_done_sync(void);
-static inline struct lll_event *prepare_dequeue_iter_ready_get(uint8_t *idx);
+static inline struct lll_event *prepare_dequeue_iter_ready_get(void **idx);
 static inline struct lll_event *resume_enqueue(lll_is_abort_cb_t is_abort_cb,
 					       lll_abort_cb_t abort_cb, lll_prepare_cb_t resume_cb,
 					       void *param);
@@ -491,9 +491,9 @@ void lll_disable(void *param)
 	}
 	{
 		struct lll_event *next;
-		uint8_t idx;
+		void *idx;
 
-		idx = UINT8_MAX;
+		idx = NULL;
 		next = ull_prepare_dequeue_iter(&idx);
 		while (next) {
 			if (!next->is_aborted &&
@@ -507,8 +507,10 @@ void lll_disable(void *param)
 				 *       the prepare pipeline hence re-iterate
 				 *       through the prepare pipeline.
 				 */
-				idx = UINT8_MAX;
+				idx = NULL;
 #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL_DONE */
+			} else if (!idx) {
+				break;
 			}
 
 			next = ull_prepare_dequeue_iter(&idx);
@@ -860,78 +862,26 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 	struct lll_event *ready_short = NULL;
 	struct lll_event *ready;
 	struct lll_event *next;
-	uint8_t idx;
+	void *idx;
 	int err;
 
 	/* Find the ready prepare in the pipeline */
-	idx = UINT8_MAX;
+	idx = NULL;
 	ready = prepare_dequeue_iter_ready_get(&idx);
 
 	/* Find any short prepare */
-	if (ready) {
-		uint32_t ticks_at_preempt_min = prepare_param->ticks_at_expire;
-		uint32_t ticks_at_preempt_next;
-		uint8_t idx_backup = idx;
+	if (ready && (&ready->prepare_param != prepare_param)) {
 		uint32_t diff;
 
-		ticks_at_preempt_next = ready->prepare_param.ticks_at_expire;
-		diff = ticker_ticks_diff_get(ticks_at_preempt_min,
-					     ticks_at_preempt_next);
-		/* If the enqueued prepare is a resume or current ready prepare is shorter, then we
-		 * should pick current ready prepare for setting up the prepare timeout.
-		 */
-		if (is_resume || ((diff & BIT(HAL_TICKER_CNTR_MSBIT)) == 0U)) {
-			ticks_at_preempt_min = ticks_at_preempt_next;
-			if (&ready->prepare_param != prepare_param) {
-				/* There is a shorter prepare in the pipeline */
-				ready_short = ready;
-			} else {
-				/* It is the same prepare in the pipeline being enqueued.
-				 * This can happen executing `lll_done()`.
-				 * Hence, we should ignore it being the `first` that setup the
-				 * preempt timeout and also it has already setup the preempt
-				 * timeout, refer to `preempt_ticker_start()` for details.
-				 *
-				 * We also set the `ready` to NULL as it is the same ready, the one
-				 * being enqueued. This help short circuit a related assertion check
-				 * later in this function.
-				 */
-				ready = NULL;
-			}
+		diff = ticker_ticks_diff_get(prepare_param->ticks_at_expire,
+					     ready->prepare_param.ticks_at_expire);
+		if (is_resume || (diff && ((diff & BIT(HAL_TICKER_CNTR_MSBIT)) == 0U))) {
+			ready_short = ready;
 		} else {
 			ready = NULL;
-			idx_backup = UINT8_MAX;
 		}
-
-		/* Loop and find any short prepare present out-of-order in the prepare pipeline.
-		 *
-		 * NOTE: This loop is O(n), where n is number of items in prepare pipeline present
-		 *       before a short prepare was enqueued in to the FIFO.
-		 *       Use of ordered linked list implementation has show improved lower latencies
-		 *       and less CPU use.
-		 * TODO: Replace use of FIFO for prepare pipeline with ordered linked list
-		 *       implementation.
-		 */
-		do {
-			struct lll_event *ready_next;
-
-			ready_next = prepare_dequeue_iter_ready_get(&idx);
-			if (!ready_next) {
-				break;
-			}
-
-			ticks_at_preempt_next = ready_next->prepare_param.ticks_at_expire;
-			diff = ticker_ticks_diff_get(ticks_at_preempt_next,
-						     ticks_at_preempt_min);
-			if ((diff & BIT(HAL_TICKER_CNTR_MSBIT)) == 0U) {
-				continue;
-			}
-
-			ready_short = ready_next;
-			ticks_at_preempt_min = ticks_at_preempt_next;
-		} while (true);
-
-		idx = idx_backup;
+	} else {
+		ready = NULL;
 	}
 
 	/* Current event active or another prepare is ready in the pipeline */
@@ -986,6 +936,8 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 				} else {
 					next = ready;
 				}
+			} else if (!idx) {
+				break;
 			}
 
 			ready = ull_prepare_dequeue_iter(&idx);
@@ -1047,6 +999,7 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
 	 */
 
 	/* Find next prepare needing preempt timeout to be setup */
+	idx = NULL;
 	next = prepare_dequeue_iter_ready_get(&idx);
 	if (!next) {
 		return err;
@@ -1083,14 +1036,20 @@ static inline bool is_done_sync(void)
 #endif /* !CONFIG_BT_CTLR_LOW_LAT_ULL_DONE */
 }
 
-static inline struct lll_event *prepare_dequeue_iter_ready_get(uint8_t *idx)
+static inline struct lll_event *prepare_dequeue_iter_ready_get(void **idx)
 {
 	struct lll_event *ready;
 
-	do {
+	ready = ull_prepare_dequeue_iter(idx);
+	while ((ready != NULL) && ((ready->is_aborted != 0U) || (ready->is_resume != 0U) ||
+				   (ready->prepare_param.defer != 0U))) {
+		if (!*idx) {
+			ready = NULL;
+			break;
+		}
+
 		ready = ull_prepare_dequeue_iter(idx);
-	} while ((ready != NULL) && ((ready->is_aborted != 0U) || (ready->is_resume != 0U) ||
-				     (ready->prepare_param.defer != 0U)));
+	}
 
 	return ready;
 }
@@ -1281,7 +1240,7 @@ static void preempt(void *param)
 {
 	lll_prepare_cb_t resume_cb;
 	struct lll_event *ready;
-	uint8_t idx;
+	void *idx;
 	int err;
 
 	/* No event to abort */
@@ -1306,9 +1265,8 @@ static void preempt(void *param)
 		return;
 	}
 
-preempt_find_preemptor:
 	/* Find a prepare that is ready and not a resume */
-	idx = UINT8_MAX;
+	idx = NULL;
 	ready = prepare_dequeue_iter_ready_get(&idx);
 	if (!ready) {
 		/* No ready prepare */
@@ -1317,87 +1275,14 @@ static void preempt(void *param)
 
 	/* Preemptor not in pipeline */
 	if (ready->prepare_param.param != param) {
-		uint32_t ticks_at_preempt_min = ready->prepare_param.ticks_at_expire;
-		struct lll_event *ready_short = NULL;
-		struct lll_event *ready_next = NULL;
-		struct lll_event *preemptor;
-
-		/* Find if the short prepare request in the pipeline */
-		do {
-			uint32_t ticks_at_preempt_next;
-			uint32_t diff;
-
-			preemptor = prepare_dequeue_iter_ready_get(&idx);
-			if (!preemptor) {
-				break;
-			}
-
-			if (!ready_next) {
-				ready_next = preemptor;
-			}
-
-			if (preemptor->prepare_param.param == param) {
-				break;
-			}
-
-			ticks_at_preempt_next = preemptor->prepare_param.ticks_at_expire;
-			diff = ticker_ticks_diff_get(ticks_at_preempt_next,
-						     ticks_at_preempt_min);
-			if ((diff & BIT(HAL_TICKER_CNTR_MSBIT)) == 0U) {
-				continue;
-			}
-
-			ready_short = preemptor;
-			ticks_at_preempt_min = ticks_at_preempt_next;
-		} while (true);
-
-		/* "The" short prepare we were looking for is not in pipeline */
-		if (!preemptor) {
-			uint32_t ret;
-
-			/* Find any short prepare */
-			if (ready_short) {
-				ready = ready_short;
-			}
-
-			/* Start the preempt timeout for (short) ready event */
-			ret = preempt_ticker_start(ready, NULL, ready);
-			LL_ASSERT_ERR((ret == TICKER_STATUS_SUCCESS) ||
-				      (ret == TICKER_STATUS_BUSY));
-
-			return;
-		}
-
-		/* FIXME: Prepare pipeline is not a ordered list implementation,
-		 *        and for short prepare being enqueued, ideally the
-		 *        pipeline has to be implemented as ordered list.
-		 *        Until then a workaround to abort a prepare present
-		 *        before the short prepare being enqueued is implemented
-		 *        below.
-		 *        A proper solution will be to re-design the pipeline
-		 *        as a ordered list, instead of the current FIFO.
-		 */
-
-		/* Abort the prepare that is present before the short prepare */
-		ready->is_aborted = 1;
-		ready->abort_cb(&ready->prepare_param, ready->prepare_param.param);
+		uint32_t ret;
 
-		/* Abort all events in pipeline before the short prepare */
-		if (preemptor != ready_next) {
-			goto preempt_find_preemptor;
-		}
+		/* Start the preempt timeout for ready event */
+		ret = preempt_ticker_start(ready, NULL, ready);
+		LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
+			  (ret == TICKER_STATUS_BUSY));
 
-		/* As the prepare queue has been refreshed due to the call of
-		 * abort_cb which invokes the lll_done, find the latest prepare
-		 */
-		idx = UINT8_MAX;
-		ready = prepare_dequeue_iter_ready_get(&idx);
-		if (!ready) {
-			/* No ready prepare */
-			return;
-		}
-
-		LL_ASSERT_ERR(ready->prepare_param.param == param);
+		return;
 	}
 
 	if (IS_ENABLED(CONFIG_BT_CTLR_LLL_PREPARE_AT_MARGIN)) {
@@ -1446,10 +1331,9 @@ static void preempt(void *param)
 	if (err == -EAGAIN) {
 		lll_is_abort_cb_t is_abort_cb;
 		lll_abort_cb_t abort_cb;
-		uint8_t is_resume_abort;
 		struct lll_event *iter;
-		uint8_t iter_idx;
 		void *curr_param;
+		void *iter_idx;
 
 		/* Remove parameter assignment from currently active radio event so that done event
 		 * is not generated.
@@ -1461,18 +1345,11 @@ static void preempt(void *param)
 		is_abort_cb = event.curr.is_abort_cb;
 		abort_cb = event.curr.abort_cb;
 
-		/* Iterate twice to ensure preempt timeout is setup after all duplicate resume
-		 * events are aborted.
-		 */
-		is_resume_abort = 0U;
-
-preempt_abort_resume:
-		/* Abort any duplicate non-resume, that they get dequeued */
-		iter_idx = UINT8_MAX;
+		/* Abort any duplicates so that they get dequeued */
+		iter_idx = NULL;
 		iter = ull_prepare_dequeue_iter(&iter_idx);
 		while (iter) {
 			if (!iter->is_aborted &&
-			    (is_resume_abort || !iter->is_resume) &&
 			    (curr_param == iter->prepare_param.param)) {
 				iter->is_aborted = 1;
 				iter->abort_cb(&iter->prepare_param,
@@ -1483,19 +1360,15 @@ static void preempt(void *param)
 				 *       the prepare pipeline hence re-iterate
 				 *       through the prepare pipeline.
 				 */
-				iter_idx = UINT8_MAX;
+				iter_idx = NULL;
 #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL_DONE */
+			} else if (!iter_idx) {
+				break;
 			}
 
 			iter = ull_prepare_dequeue_iter(&iter_idx);
 		}
 
-		if (!is_resume_abort) {
-			is_resume_abort = 1U;
-
-			goto preempt_abort_resume;
-		}
-
 		/* Enqueue as resume event */
 		iter = resume_enqueue(is_abort_cb, abort_cb, resume_cb, curr_param);
 		LL_ASSERT_ERR(iter);