From f6a3da9c21806364be6c9fd98079204c5f2b28f6 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Tue, 2 Aug 2022 11:20:39 +0530 Subject: [PATCH] Bluetooth: Controller: Review rework of jitter in aux offset Review rework of the fix to remove the jitter in aux offset and sync offset values. Force select BT_TICKER_REMAINDER_GET and BT_TICKER_LAZY_GET features when Extended Advertising and Periodic Advertising is supported. Rename ticks and microsecond offset value struct members for primary PDU event offset (to auxiliary PDU event). Converted HAL_TICKER_REMOVE_JITTER and HAL_TICKER_ADD_JITTER macro to functions. Signed-off-by: Vinayak Kariappa Chettimada --- .../bluetooth/controller/Kconfig.ll_sw_split | 5 +- subsys/bluetooth/controller/ll_sw/lll_adv.h | 4 +- .../ll_sw/nordic/hal/nrf5/radio/radio.c | 2 +- .../controller/ll_sw/nordic/hal/nrf5/ticker.h | 68 +++++++++++-------- .../controller/ll_sw/nordic/lll/lll_adv.c | 4 +- .../bluetooth/controller/ll_sw/ull_adv_aux.c | 8 +-- .../bluetooth/controller/ll_sw/ull_adv_sync.c | 10 ++- 7 files changed, 60 insertions(+), 41 deletions(-) diff --git a/subsys/bluetooth/controller/Kconfig.ll_sw_split b/subsys/bluetooth/controller/Kconfig.ll_sw_split index 5f96054497..9550909338 100644 --- a/subsys/bluetooth/controller/Kconfig.ll_sw_split +++ b/subsys/bluetooth/controller/Kconfig.ll_sw_split @@ -43,6 +43,9 @@ config BT_LLL_VENDOR_NORDIC select BT_CTLR_TIFS_HW_SUPPORT select BT_CTLR_ULL_LLL_PRIO_SUPPORT + select BT_TICKER_REMAINDER_GET if BT_BROADCASTER && BT_CTLR_ADV_EXT + select BT_TICKER_LAZY_GET if BT_CTLR_ADV_PERIODIC + default y help Use Nordic Lower Link Layer implementation. @@ -709,7 +712,6 @@ config BT_TICKER_LOW_LAT config BT_TICKER_REMAINDER_GET bool "Ticker Next Slot Get with Remainder" - default y if BT_BROADCASTER && BT_CTLR_ADV_EXT help This option enables ticker interface to iterate through active ticker nodes, returning tick to expire and remainder from a reference @@ -717,7 +719,6 @@ config BT_TICKER_REMAINDER_GET config BT_TICKER_LAZY_GET bool "Ticker Next Slot Get with Lazy" - default y if BT_CTLR_ADV_PERIODIC help This option enables ticker interface to iterate through active ticker nodes, returning tick to expire and lazy count from a reference diff --git a/subsys/bluetooth/controller/ll_sw/lll_adv.h b/subsys/bluetooth/controller/ll_sw/lll_adv.h index 2a108b0570..fee4293d1a 100644 --- a/subsys/bluetooth/controller/ll_sw/lll_adv.h +++ b/subsys/bluetooth/controller/ll_sw/lll_adv.h @@ -130,8 +130,8 @@ struct lll_adv_aux { /* Store used by primary channel PDU event to fill the * auxiliary offset to this auxiliary PDU event. */ - uint32_t ticks_offset; - uint32_t us_offset; + uint32_t ticks_pri_pdu_offset; + uint32_t us_pri_pdu_offset; struct lll_adv_pdu data; #if defined(CONFIG_BT_CTLR_ADV_PDU_LINK) diff --git a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c index c451ab02a3..88e7ad6da4 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c @@ -1050,7 +1050,7 @@ void radio_tmr_tifs_set(uint32_t tifs) uint32_t radio_tmr_start(uint8_t trx, uint32_t ticks_start, uint32_t remainder) { - HAL_TICKER_REMOVE_JITTER(ticks_start, remainder); + hal_ticker_remove_jitter(&ticks_start, &remainder); nrf_timer_task_trigger(EVENT_TIMER, NRF_TIMER_TASK_CLEAR); EVENT_TIMER->MODE = 0; diff --git a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/ticker.h b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/ticker.h index 8ffcf0a8e4..5845d15145 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/ticker.h +++ b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/ticker.h @@ -6,6 +6,7 @@ */ #define HAL_TICKER_CNTR_CLK_FREQ_HZ 32768U +#define HAL_TICKER_CNTR_CLK_UNIT_FS 30517578125UL /* Macro defining the minimum counter compare offset */ #define HAL_TICKER_CNTR_CMP_OFFSET_MIN 3 @@ -24,44 +25,28 @@ */ #define HAL_TICKER_US_TO_TICKS(x) \ ( \ - ((uint32_t)(((uint64_t) (x) * 1000000000UL) / 30517578125UL)) \ - & HAL_TICKER_CNTR_MASK \ + ((uint32_t)(((uint64_t) (x) * 1000000000UL) / \ + HAL_TICKER_CNTR_CLK_UNIT_FS)) & HAL_TICKER_CNTR_MASK \ ) -/* Macro returning remainder in picoseconds */ +/* Macro to translate tick units to microseconds. */ +#define HAL_TICKER_TICKS_TO_US(x) \ + ( \ + ((uint32_t)(((uint64_t)(x) * HAL_TICKER_CNTR_CLK_UNIT_FS) / \ + 1000000000UL)) \ + ) + +/* Macro returning remainder in picoseconds (to fit in 32-bits) */ #define HAL_TICKER_REMAINDER(x) \ ( \ ( \ ((uint64_t) (x) * 1000000000UL) \ - - ((uint64_t)HAL_TICKER_US_TO_TICKS(x) * 30517578125UL) \ + - ((uint64_t)HAL_TICKER_US_TO_TICKS(x) * \ + HAL_TICKER_CNTR_CLK_UNIT_FS) \ ) \ / 1000UL \ ) -/* Macro to remove ticks and return positive remainder value in microseconds */ -#define HAL_TICKER_REMOVE_JITTER(t, r) \ - { \ - if ((!(r / 1000000UL)) || (r & BIT(31))) { \ - t--; \ - r += 30517578UL; \ - } \ - r /= 1000000UL; \ - } - -/* Macro to add ticks and return positive remainder value in microseconds */ -#define HAL_TICKER_ADD_JITTER(t, r) \ - { \ - if ((!(r / 1000000UL)) || (r & BIT(31))) { \ - t++; \ - r += 30517578UL; \ - } \ - r /= 1000000UL; \ - } - -/* Macro to translate tick units to microseconds. */ -#define HAL_TICKER_TICKS_TO_US(x) \ - ((uint32_t)(((uint64_t)(x) * 30517578125UL) / 1000000000UL)) - /* Macro defining the remainder resolution/range * ~ 1000000 * HAL_TICKER_TICKS_TO_US(1) */ @@ -71,3 +56,30 @@ /* Macro defining the margin for positioning re-scheduled nodes */ #define HAL_TICKER_RESCHEDULE_MARGIN \ HAL_TICKER_US_TO_TICKS(150) + +/* Remove ticks and return positive remainder value in microseconds */ +static inline void hal_ticker_remove_jitter(uint32_t *ticks, + uint32_t *remainder) +{ + /* Is remainder less than 1 us */ + if ((*remainder & BIT(31)) || !(*remainder / 1000000UL)) { + *ticks -= 1U; + *remainder += HAL_TICKER_CNTR_CLK_UNIT_FS / 1000UL; + } + + /* pico seconds to micro seconds unit */ + *remainder /= 1000000UL; +} + +/* Add ticks and return positive remainder value in microseconds */ +static inline void hal_ticker_add_jitter(uint32_t *ticks, uint32_t *remainder) +{ + /* Is remainder less than 1 us */ + if ((*remainder & BIT(31)) || !(*remainder / 1000000UL)) { + *ticks += 1U; + *remainder += HAL_TICKER_CNTR_CLK_UNIT_FS / 1000UL; + } + + /* pico seconds to micro seconds unit */ + *remainder /= 1000000UL; +} diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c index e16591968e..da079c2971 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c @@ -1316,8 +1316,8 @@ static void isr_done(void *param) lll_aux = lll->aux; if (lll_aux) { (void)ull_adv_aux_lll_offset_fill(pdu, - lll_aux->ticks_offset, - lll_aux->us_offset, + lll_aux->ticks_pri_pdu_offset, + lll_aux->us_pri_pdu_offset, start_us); } #else /* !CONFIG_BT_CTLR_ADV_EXT */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c b/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c index bd85d4b74d..aeebf74905 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c +++ b/subsys/bluetooth/controller/ll_sw/ull_adv_aux.c @@ -1503,24 +1503,24 @@ static void mfy_aux_offset_get(void *param) } while (id != ticker_id); /* Adjust ticks to expire based on remainder value */ - HAL_TICKER_REMOVE_JITTER(ticks_to_expire, remainder); + hal_ticker_remove_jitter(&ticks_to_expire, &remainder); /* Store the ticks offset for population in other advertising primary * channel PDUs. */ - lll_aux->ticks_offset = ticks_to_expire; + lll_aux->ticks_pri_pdu_offset = ticks_to_expire; /* NOTE: as first primary channel PDU does not use remainder, the packet * timer is started one tick in advance to start the radio with * microsecond precision, hence compensate for the higher start_us value * captured at radio start of the first primary channel PDU. */ - lll_aux->ticks_offset += 1U; + lll_aux->ticks_pri_pdu_offset += 1U; /* Store the microsecond remainder offset for population in other * advertising primary channel PDUs. */ - lll_aux->us_offset = remainder; + lll_aux->us_pri_pdu_offset = remainder; /* Fill the aux offset in the first Primary channel PDU */ /* FIXME: we are in ULL_LOW context, fill offset in LLL context? */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c b/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c index 415d826678..c7f7ff59fb 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c +++ b/subsys/bluetooth/controller/ll_sw/ull_adv_sync.c @@ -1831,11 +1831,17 @@ static void mfy_sync_offset_get(void *param) LL_ASSERT(id != TICKER_NULL); } while (id != ticker_id); - HAL_TICKER_REMOVE_JITTER(ticks_to_expire, remainder); + /* Reduced a tick for negative remainder and return positive remainder + * value. + */ + hal_ticker_remove_jitter(&ticks_to_expire, &remainder); sync_remainder_us = remainder; + /* Add a tick for negative remainder and return positive remainder + * value. + */ remainder = sync->aux_remainder; - HAL_TICKER_ADD_JITTER(ticks_to_expire, remainder); + hal_ticker_add_jitter(&ticks_to_expire, &remainder); aux_remainder_us = remainder; pdu = lll_adv_aux_data_latest_peek(adv->lll.aux);