Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: Controller: Replace prepare pipeline with ordered list #79444

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions subsys/bluetooth/controller/Kconfig.ll_sw_split
Original file line number Diff line number Diff line change
Expand Up @@ -1083,14 +1083,6 @@ config BT_CTLR_SCAN_UNRESERVED
Scanner will not use time space reservation for scan window when in
continuous scan mode.

config BT_CTLR_EARLY_ABORT_PREVIOUS_PREPARE
bool "Early abort previous prepare"
depends on !BT_CTLR_LOW_LAT
default y
help
Early abort previous prepare present before a short prepare is
enqueued in the prepare pipeline.

config BT_MAYFLY_YIELD_AFTER_CALL
bool "Yield from mayfly thread after first call"
default y
Expand Down
2 changes: 1 addition & 1 deletion subsys/bluetooth/controller/ll_sw/lll.h
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,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);
Expand Down
18 changes: 11 additions & 7 deletions subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1349,10 +1349,12 @@ uint32_t radio_tmr_start(uint8_t trx, uint32_t ticks_start, uint32_t remainder)
* by the Radio ISR, the compare will trigger again.
*/
uint32_t latency_ticks;
uint32_t latency_us;

latency_ticks = HAL_TICKER_US_TO_TICKS(HAL_RADIO_ISR_LATENCY_MAX_US);
latency_us = MAX(remainder, HAL_RADIO_ISR_LATENCY_MAX_US) - remainder;
latency_ticks = HAL_TICKER_US_TO_TICKS(latency_us);
ticks_start -= latency_ticks;
remainder += HAL_TICKER_TICKS_TO_US(latency_ticks);
remainder += latency_us;
#endif /* !CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER */

nrf_timer_task_trigger(EVENT_TIMER, NRF_TIMER_TASK_CLEAR);
Expand Down Expand Up @@ -1473,10 +1475,12 @@ uint32_t radio_tmr_start_tick(uint8_t trx, uint32_t ticks_start)
* by the Radio ISR, the compare will trigger again.
*/
uint32_t latency_ticks;
uint32_t latency_us;

latency_ticks = HAL_TICKER_US_TO_TICKS(HAL_RADIO_ISR_LATENCY_MAX_US);
latency_us = MAX(remainder_us, HAL_RADIO_ISR_LATENCY_MAX_US) - remainder_us;
latency_ticks = HAL_TICKER_US_TO_TICKS(latency_us);
ticks_start -= latency_ticks;
remainder_us += HAL_TICKER_TICKS_TO_US(latency_ticks);
remainder_us += latency_us;
#endif /* !CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER */

nrf_timer_task_trigger(EVENT_TIMER, NRF_TIMER_TASK_STOP);
Expand Down Expand Up @@ -1614,10 +1618,10 @@ uint32_t radio_tmr_start_us(uint8_t trx, uint32_t start_us)
* The timer is cleared on Radio End and if the PPI/DPPI is not disabled
* by the Radio ISR, the compare will trigger again.
*/
uint32_t latency_ticks;
uint32_t latency_us;

latency_ticks = HAL_TICKER_US_TO_TICKS(HAL_RADIO_ISR_LATENCY_MAX_US);
actual_us += HAL_TICKER_TICKS_TO_US(latency_ticks);
latency_us = MAX(actual_us, HAL_RADIO_ISR_LATENCY_MAX_US) - actual_us;
actual_us += latency_us;
#endif /* !CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER */

nrf_timer_event_clear(EVENT_TIMER, NRF_TIMER_EVENT_COMPARE0);
Expand Down
157 changes: 85 additions & 72 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
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_prepare_cb_t resume_cb);
static void isr_race(void *param);

Expand Down Expand Up @@ -404,9 +404,9 @@
}
{
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 &&
Expand All @@ -420,8 +420,10 @@
* 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);
Expand Down Expand Up @@ -778,21 +780,36 @@
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 = ready->prepare_param.ticks_at_expire;
uint32_t ticks_at_preempt_min = prepare_param->ticks_at_expire;
uint32_t ticks_at_preempt_next;
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 (is_resume ||
(diff && ((diff & BIT(HAL_TICKER_CNTR_MSBIT)) == 0U))) {
ticks_at_preempt_min = ticks_at_preempt_next;

Check notice on line 801 in subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c:801 - diff = ticker_ticks_diff_get(ticks_at_preempt_min, - ticks_at_preempt_next); - if (is_resume || - (diff && ((diff & BIT(HAL_TICKER_CNTR_MSBIT)) == 0U))) { + diff = ticker_ticks_diff_get(ticks_at_preempt_min, ticks_at_preempt_next); + if (is_resume || (diff && ((diff & BIT(HAL_TICKER_CNTR_MSBIT)) == 0U))) {
ready_short = ready;
} else {
ready = NULL;
}

do {
uint32_t ticks_at_preempt_next;
struct lll_event *ready_next;
uint32_t diff;

if (!idx) {
break;
}

ready_next = prepare_dequeue_iter_ready_get(&idx);
if (!ready_next) {
Expand Down Expand Up @@ -859,6 +876,8 @@
} else {
next = ready;
}
} else if (!idx) {
break;
}

ready = ull_prepare_dequeue_iter(&idx);
Expand Down Expand Up @@ -907,6 +926,7 @@
*/

/* Find next prepare needing preempt timeout to be setup */
idx = NULL;
next = prepare_dequeue_iter_ready_get(&idx);
if (!next) {
return err;
Expand Down Expand Up @@ -943,13 +963,19 @@
#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 && (ready->is_aborted || ready->is_resume)) {
if (!*idx) {
ready = NULL;
break;
}

ready = ull_prepare_dequeue_iter(idx);
} while (ready && (ready->is_aborted || ready->is_resume));
}

return ready;
}
Expand Down Expand Up @@ -1066,32 +1092,6 @@
LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
(ret == TICKER_STATUS_BUSY));

#if defined(CONFIG_BT_CTLR_EARLY_ABORT_PREVIOUS_PREPARE)
/* 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.
*/
/* preempt timeout already started but no role/state in the head
* of prepare pipeline.
*/
if (prev && !prev->is_aborted) {
/* Set early as we get called again through the call to
* abort_cb().
*/
ticks_at_preempt = ticks_at_preempt_new;

/* Abort previous prepare that set the preempt timeout */
prev->is_aborted = 1U;
prev->abort_cb(&prev->prepare_param,
prev->prepare_param.param);
}
#endif /* CONFIG_BT_CTLR_EARLY_ABORT_PREVIOUS_PREPARE */

/* Schedule short preempt timeout */
first = next;
} else {
Expand Down Expand Up @@ -1172,7 +1172,7 @@
{
lll_prepare_cb_t resume_cb;
struct lll_event *ready;
uint8_t idx;
void *idx;
int err;

/* No event to abort */
Expand All @@ -1181,7 +1181,7 @@
}

/* 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 */
Expand All @@ -1190,51 +1190,62 @@

/* 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;
uint32_t ret;

/* Find if a short prepare request in the pipeline */
/* Find if the short prepare request in the pipeline */
do {
preemptor = ull_prepare_dequeue_iter(&idx);
if (!ready_next && preemptor && !preemptor->is_aborted &&
!preemptor->is_resume) {
uint32_t ticks_at_preempt_next;
uint32_t diff;

if (!idx) {
preemptor = NULL;
break;
}

preemptor = prepare_dequeue_iter_ready_get(&idx);
if (!preemptor) {
break;
}

if (!ready_next) {
ready_next = preemptor;
}
} while (preemptor && (preemptor->is_aborted || preemptor->is_resume ||
(preemptor->prepare_param.param != param)));

/* No short prepare request in pipeline */
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) {

Check notice on line 1225 in subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c:1225 - diff = ticker_ticks_diff_get(ticks_at_preempt_next, - ticks_at_preempt_min); + diff = ticker_ticks_diff_get(ticks_at_preempt_next, ticks_at_preempt_min);
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) {
/* Start the preempt timeout for ready event */
/* 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((ret == TICKER_STATUS_SUCCESS) ||
(ret == TICKER_STATUS_BUSY));

return;
}

/* FIXME: Abort all events in pipeline before the short
* prepare event. For now, lets assert when many
* enqueued prepares need aborting.
*/
LL_ASSERT(preemptor == ready_next);

/* Abort the prepare that is present before the short prepare */
ready->is_aborted = 1;
ready->abort_cb(&ready->prepare_param, ready->prepare_param.param);

/* 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(ready->prepare_param.param == param);
ready = preemptor;
}

/* Check if current event want to continue */
Expand All @@ -1253,10 +1264,10 @@
/* Check if resume requested */
if (err == -EAGAIN) {
struct lll_event *iter;
uint8_t iter_idx;
void *iter_idx;

/* Abort any duplicates so that they get dequeued */
iter_idx = UINT8_MAX;
iter_idx = NULL;
iter = ull_prepare_dequeue_iter(&iter_idx);
while (iter) {
if (!iter->is_aborted &&
Expand All @@ -1270,8 +1281,10 @@
* 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);
Expand Down
Loading
Loading