Skip to content

Issue Fixes for Bap Broadcast Samples and Enable LC3 Codec For iMXRT1062 MCU #93091

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion samples/bluetooth/bap_broadcast_sink/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ config ENABLE_LC3
# By default let's enable it in the platforms we know are capable of supporting it
default y
depends on CPU_HAS_FPU && \
(ARCH_POSIX || SOC_COMPATIBLE_NRF52X || SOC_COMPATIBLE_NRF5340_CPUAPP)
(ARCH_POSIX || SOC_COMPATIBLE_NRF52X || SOC_COMPATIBLE_NRF5340_CPUAPP || SOC_MIMXRT1062)
select LIBLC3
select FPU

Expand Down
2 changes: 1 addition & 1 deletion samples/bluetooth/bap_broadcast_source/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ config ENABLE_LC3
# By default let's enable it in the platforms we know are capable of supporting it
default y
depends on CPU_HAS_FPU && \
(ARCH_POSIX || SOC_COMPATIBLE_NRF52X || SOC_COMPATIBLE_NRF5340_CPUAPP)
(ARCH_POSIX || SOC_COMPATIBLE_NRF52X || SOC_COMPATIBLE_NRF5340_CPUAPP || SOC_MIMXRT1062)
select LIBLC3
select FPU

Expand Down
13 changes: 13 additions & 0 deletions subsys/bluetooth/common/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,19 @@ config BT_BUF_EVT_DISCARDABLE_COUNT
it will not cause the allocation for other critical events to
block and may even eliminate deadlocks in some cases.

config BT_BUF_EVT_SYNCHRONOUS_COUNT
int "Number of synchronous HCI Event buffers"
range 1 $(UINT8_MAX)
# number of buffer require for bap-broadcast BIS counts
default BT_BAP_BROADCAST_SRC_STREAM_COUNT if BT_BAP_BROADCAST_SOURCE
default 1
help
Number of buffers in a separate pool for events which the HCI
stack depedent to manage it's command buffer queue, host to
controller buffer credits etc. The events command_complete,
number of command processed(nocp) etc considered under this
event pool and processed on priority.

config BT_BUF_CMD_TX_SIZE
int "Maximum support HCI Command buffer length"
default $(UINT8_MAX) if (BT_EXT_ADV || BT_CLASSIC || BT_ISO_CENTRAL || BT_CHANNEL_SOUNDING)
Expand Down
3 changes: 2 additions & 1 deletion subsys/bluetooth/host/buf.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ static void iso_rx_freed_cb(void)
* the HCI transport to fill buffers in parallel with `bt_recv`
* consuming them.
*/
NET_BUF_POOL_FIXED_DEFINE(sync_evt_pool, 1, SYNC_EVT_SIZE, 0, NULL);
NET_BUF_POOL_FIXED_DEFINE(sync_evt_pool, CONFIG_BT_BUF_EVT_SYNCHRONOUS_COUNT, SYNC_EVT_SIZE, 0,
NULL);
Comment on lines +67 to +68
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/* Pool for RX HCI buffers that are always freed by `bt_recv`
 * before it returns.
 *
 * A singleton buffer shall be sufficient for correct operation.
 * The buffer count may be increased as an optimization to allow
 * the HCI transport to fill buffers in parallel with `bt_recv`
 * consuming them.
 */

Based on the comment in the implementation, and the fact that bt_recv control flow has implicit event flow control; I do not agree that this PR fixes any bug. Increasing the buffer count is an optimization at the cost of increased RAM usage, as each buffer can be over "quarter of a kilobyte" in size!

HCI Number of Completed Packets event "can" accumulate the counts for multiple handles and be generated at a frequency at the implementations discretion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @cvinayak, thanks for your comment.

I agree that this is not a bug, but the issue at host exposes with some controller if it sends NOCP events for all BISes under one subgroup at the end of subgroup event.

I need your help to understand how existing implementation without fix in current PR would fix credit related issues in broadcast tests with multiple BISes where host gets NOCP of each BISes within gap of short duration?

Also, this will not cause any memory increase in normal tests, if the test is specific for multi stream broadcast then it needs stream specific buffer counts to receive NOCP for each bis-handle (may be together or with some gap within ISO).

Regards,
Nirav


NET_BUF_POOL_FIXED_DEFINE(discardable_pool, CONFIG_BT_BUF_EVT_DISCARDABLE_COUNT,
BT_BUF_EVT_SIZE(CONFIG_BT_BUF_EVT_DISCARDABLE_SIZE),
Expand Down