Skip to content

Commit

Permalink
Bluetooth: Controller: Add margin to ISOALs time offset
Browse files Browse the repository at this point in the history
To ensure payloads are delivered in time for the first subevent
in framed BIS, ISOAL now enforces a (configurable) margin of
the calculated time offset

Without this margin, it has been observed that a broadcaster
can end up consistently missing the first subevent in every third
event in a 7.5 ms ISO with a 10 ms SDU interval

The margin is a conservative 2 ms by default, but can likely be
set a lot lower for most implementations and HWs

Signed-off-by: Troels Nilsson <[email protected]>
  • Loading branch information
Tronil authored and nashif committed Nov 16, 2024
1 parent 99a5236 commit fa3bfa5
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
19 changes: 19 additions & 0 deletions subsys/bluetooth/controller/Kconfig.ll_sw_split
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,25 @@ config BT_CTLR_ISOAL_PSN_IGNORE
help
Ignore the use of Tx ISO Data Packet Sequence Number.

config BT_CTLR_ISOAL_FRAMED_BIS_OFFSET_MARGIN
int "Margin (in microseconds) to be used in framed time offset for BIS"
depends on BT_CTLR_ADV_ISO || BT_CTLR_CONN_ISO
default 2000
range 0 10000
help
Needed margin for reliable delivery of payloads will vary, but should
generally be EVENT_OVERHEAD_START_US + a small margin to cover ISOAL
processing overhead

config BT_CTLR_ISOAL_FRAMED_CIS_OFFSET_MARGIN
int "Margin (in microseconds) to be used in framed time offset for CIS"
depends on BT_CTLR_ADV_ISO || BT_CTLR_CONN_ISO
default 0
range 0 10000
help
Note: Usually no margin is needed for CIS as null PDUs can be used if a payload
is too late for the first subevent

config BT_CTLR_ZLI
bool "Use Zero Latency IRQs"
depends on ZERO_LATENCY_IRQS
Expand Down
10 changes: 7 additions & 3 deletions subsys/bluetooth/controller/ll_sw/isoal.c
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ static isoal_status_t isoal_check_source_hdl_valid(isoal_source_handle_t hdl)
* @param pdu_release[in] Callback of PDU deallocator
* @param hdl[out] Handle to new source
*
* @return ISOAL_STATUS_OK if we could create a new sink; otherwise ISOAL_STATUS_ERR_SOURCE_ALLOC
* @return ISOAL_STATUS_OK if we could create a new source; otherwise ISOAL_STATUS_ERR_SOURCE_ALLOC
*/
isoal_status_t isoal_source_create(
uint16_t handle,
Expand Down Expand Up @@ -1550,6 +1550,7 @@ isoal_status_t isoal_source_create(

session->handle = handle;
session->framed = framed;
session->bis = role == ISOAL_ROLE_BROADCAST_SOURCE;
session->burst_number = burst_number;
session->iso_interval = iso_interval;
session->sdu_interval = sdu_interval;
Expand Down Expand Up @@ -2343,6 +2344,9 @@ static uint16_t isoal_tx_framed_find_correct_tx_event(const struct isoal_source
const bool time_stamp_is_valid = isoal_is_time_stamp_valid(source_ctx,
tx_sdu->cntr_time_stamp,
tx_sdu->time_stamp);
const uint16_t offset_margin = session->bis ?
CONFIG_BT_CTLR_ISOAL_FRAMED_BIS_OFFSET_MARGIN :
CONFIG_BT_CTLR_ISOAL_FRAMED_CIS_OFFSET_MARGIN;

/* Adjust payload number */
if (pp->initialized) {
Expand Down Expand Up @@ -2437,7 +2441,7 @@ static uint16_t isoal_tx_framed_find_correct_tx_event(const struct isoal_source
* The Time_Offset shall be a positive value.
*/
while (!isoal_get_time_diff(time_stamp_selected, actual_grp_ref_point, &time_diff)
|| time_diff == 0) {
|| time_diff <= offset_margin) {
/* Advance target to next event */
actual_event++;
actual_grp_ref_point = isoal_get_wrapped_time_us(actual_grp_ref_point,
Expand Down Expand Up @@ -2525,7 +2529,7 @@ static isoal_status_t isoal_tx_framed_produce(isoal_source_handle_t source_hdl,
uint64_t next_payload_number;
uint16_t sdus_skipped;
bool time_diff_valid;
uint32_t time_diff;
uint32_t time_diff = 0U;

/* Start of a new SDU */
time_diff_valid = isoal_get_time_diff(session->last_input_time_stamp,
Expand Down
3 changes: 2 additions & 1 deletion subsys/bluetooth/controller/ll_sw/isoal.h
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,8 @@ struct isoal_source_session {
uint32_t sdu_interval;
uint16_t handle;
uint16_t iso_interval;
uint8_t framed;
uint8_t framed: 1;
uint8_t bis: 1;
uint8_t burst_number;
uint8_t pdus_per_sdu;
uint8_t max_pdu_size;
Expand Down
19 changes: 19 additions & 0 deletions tests/bluetooth/controller/ctrl_isoal/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,25 @@ config BT_CTLR_ISOAL_SN_STRICT
depends on BT_CTLR_ADV_ISO || BT_CTLR_CONN_ISO
default y

config BT_CTLR_ISOAL_FRAMED_BIS_OFFSET_MARGIN
int "Margin (in microseconds) to be used in framed time offset for BIS"
depends on BT_CTLR_ADV_ISO || BT_CTLR_CONN_ISO
default 2000
range 0 10000
help
Needed margin for reliable delivery of payloads will vary, but should
generally be EVENT_OVERHEAD_START_US + a small margin to cover ISOAL
processing overhead

config BT_CTLR_ISOAL_FRAMED_CIS_OFFSET_MARGIN
int "Margin (in microseconds) to be used in framed time offset for CIS"
depends on BT_CTLR_ADV_ISO || BT_CTLR_CONN_ISO
default 0
range 0 10000
help
Note: Usually no margin is needed for CIS as Null PDUs can be used if a payload
is too late for the first subevent

source "tests/bluetooth/controller/common/Kconfig"

source "Kconfig.zephyr"

0 comments on commit fa3bfa5

Please sign in to comment.