From 1b548bb34a0bfc300b8aa03c271148141673922f Mon Sep 17 00:00:00 2001 From: Guang Xie Date: Thu, 15 Feb 2018 12:42:15 -0800 Subject: [PATCH] Enable HCI Sniff mode and disable subrating for A2DP Enabling the HCI Sniff mode could help headsets to save power. Note that subrating is disabled implicitly by NOT sending the "Sent Sniff Subrating" HCI command to the BT controller, and effectively using the default Sniff Subrating parameters. The Headset shouldn't go into Subrating Mode to prevent potential stuttering if the Headset chooses to use subrating with the wrong parameters. NOTE: This should affect only Pixel Buds. Also: - Added some extra log messages - Minor cleanup (argument/variable renaming) Bug: 74344711 Bug: 74361002 Test: Manual - audio streaming; examine btsnoop logs Change-Id: Iec5086dba6f9003c21bcb41eddb3bc83ac7dddd1 --- bta/av/bta_av_aact.cc | 51 ++++++++++++++++++++++++++------------------- bta/av/bta_av_main.cc | 10 ++++----- bta/dm/bta_dm_cfg.cc | 2 +- bta/sys/bta_sys.h | 6 +++--- bta/sys/bta_sys_conn.cc | 16 +++++++++----- internal_include/bt_trace.h | 12 +++++++++++ 6 files changed, 62 insertions(+), 35 deletions(-) diff --git a/bta/av/bta_av_aact.cc b/bta/av/bta_av_aact.cc index af80b0633..89d9523e5 100644 --- a/bta/av/bta_av_aact.cc +++ b/bta/av/bta_av_aact.cc @@ -1832,7 +1832,7 @@ void bta_av_conn_failed(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { * ******************************************************************************/ void bta_av_do_start(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { - uint8_t policy = HCI_ENABLE_SNIFF_MODE; + uint8_t clear_policy = 0; uint8_t cur_role; APPL_TRACE_DEBUG("%s: sco_occupied:%d, role:0x%x, started:%d", __func__, @@ -1847,10 +1847,10 @@ void bta_av_do_start(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { * It would not hurt us, if the peer device wants us to be master */ if ((BTM_GetRole(p_scb->PeerAddress(), &cur_role) == BTM_SUCCESS) && (cur_role == BTM_ROLE_MASTER)) { - policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; + clear_policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; } - bta_sys_clear_policy(BTA_ID_AV, policy, p_scb->PeerAddress()); + bta_sys_clear_policy(BTA_ID_AV, clear_policy, p_scb->PeerAddress()); if ((!p_scb->started) && ((p_scb->role & BTA_AV_ROLE_START_INT) == 0)) { p_scb->role |= BTA_AV_ROLE_START_INT; @@ -1885,7 +1885,7 @@ void bta_av_str_stopped(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { uint8_t start = p_scb->started; bool sus_evt = true; BT_HDR* p_buf; - uint8_t policy = HCI_ENABLE_SNIFF_MODE; + uint8_t set_policy = HCI_ENABLE_SNIFF_MODE; APPL_TRACE_ERROR( "%s: peer %s handle:%d audio_open_cnt:%d, p_data %p start:%d", __func__, @@ -1894,9 +1894,10 @@ void bta_av_str_stopped(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { bta_sys_idle(BTA_ID_AV, bta_av_cb.audio_open_cnt, p_scb->PeerAddress()); if ((bta_av_cb.features & BTA_AV_FEAT_MASTER) == 0 || - bta_av_cb.audio_open_cnt == 1) - policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; - bta_sys_set_policy(BTA_ID_AV, policy, p_scb->PeerAddress()); + bta_av_cb.audio_open_cnt == 1) { + set_policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; + } + bta_sys_set_policy(BTA_ID_AV, set_policy, p_scb->PeerAddress()); if (p_scb->co_started) { if (p_scb->offload_started) { @@ -2190,7 +2191,7 @@ void bta_av_start_ok(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { uint16_t flush_to; uint8_t new_role = p_scb->role; BT_HDR hdr; - uint8_t policy = HCI_ENABLE_SNIFF_MODE; + uint8_t clear_policy = 0; uint8_t cur_role; uint8_t local_tsep = p_scb->seps[p_scb->sep_idx].tsep; @@ -2327,10 +2328,10 @@ void bta_av_start_ok(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { * master */ if ((BTM_GetRole(p_scb->PeerAddress(), &cur_role) == BTM_SUCCESS) && (cur_role == BTM_ROLE_MASTER)) { - policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; + clear_policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; } - bta_sys_clear_policy(BTA_ID_AV, policy, p_scb->PeerAddress()); + bta_sys_clear_policy(BTA_ID_AV, clear_policy, p_scb->PeerAddress()); } p_scb->role = new_role; @@ -2381,14 +2382,20 @@ void bta_av_start_ok(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { * ******************************************************************************/ void bta_av_start_failed(tBTA_AV_SCB* p_scb, UNUSED_ATTR tBTA_AV_DATA* p_data) { + uint8_t set_policy = (HCI_ENABLE_SNIFF_MODE | HCI_ENABLE_MASTER_SLAVE_SWITCH); + + APPL_TRACE_ERROR( + "%s: peer %s handle:%d audio_open_cnt:%d started:%s co_started:%d", + __func__, p_scb->PeerAddress().ToString().c_str(), p_scb->hndl, + bta_av_cb.audio_open_cnt, logbool(p_scb->started).c_str(), + p_scb->co_started); + if (!p_scb->started && !p_scb->co_started) { bta_sys_idle(BTA_ID_AV, bta_av_cb.audio_open_cnt, p_scb->PeerAddress()); notify_start_failed(p_scb); } - bta_sys_set_policy(BTA_ID_AV, - (HCI_ENABLE_SNIFF_MODE | HCI_ENABLE_MASTER_SLAVE_SWITCH), - p_scb->PeerAddress()); + bta_sys_set_policy(BTA_ID_AV, set_policy, p_scb->PeerAddress()); p_scb->sco_suspend = false; } @@ -2404,7 +2411,7 @@ void bta_av_start_failed(tBTA_AV_SCB* p_scb, UNUSED_ATTR tBTA_AV_DATA* p_data) { void bta_av_str_closed(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { tBTA_AV data; tBTA_AV_EVT event; - uint8_t policy = HCI_ENABLE_SNIFF_MODE; + uint8_t set_policy = HCI_ENABLE_SNIFF_MODE; APPL_TRACE_WARNING( "%s: peer %s handle:%d open_status:%d chnl:%d co_started:%d", __func__, @@ -2412,9 +2419,10 @@ void bta_av_str_closed(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { p_scb->chnl, p_scb->co_started); if ((bta_av_cb.features & BTA_AV_FEAT_MASTER) == 0 || - bta_av_cb.audio_open_cnt == 1) - policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; - bta_sys_set_policy(BTA_ID_AV, policy, p_scb->PeerAddress()); + bta_av_cb.audio_open_cnt == 1) { + set_policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; + } + bta_sys_set_policy(BTA_ID_AV, set_policy, p_scb->PeerAddress()); if (bta_av_cb.audio_open_cnt <= 1) { /* last connection - restore the allow switch flag */ L2CA_SetDesireRole(L2CAP_ROLE_ALLOW_SWITCH); @@ -2485,7 +2493,7 @@ void bta_av_clr_cong(tBTA_AV_SCB* p_scb, UNUSED_ATTR tBTA_AV_DATA* p_data) { void bta_av_suspend_cfm(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { tBTA_AV_SUSPEND suspend_rsp; uint8_t err_code = p_data->str_msg.msg.hdr.err_code; - uint8_t policy = HCI_ENABLE_SNIFF_MODE; + uint8_t set_policy = HCI_ENABLE_SNIFF_MODE; APPL_TRACE_DEBUG("%s: peer %s handle:%d audio_open_cnt:%d err_code:%d", __func__, p_scb->PeerAddress().ToString().c_str(), @@ -2525,9 +2533,10 @@ void bta_av_suspend_cfm(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) { bta_sys_idle(BTA_ID_AV, bta_av_cb.audio_open_cnt, p_scb->PeerAddress()); if ((bta_av_cb.features & BTA_AV_FEAT_MASTER) == 0 || - bta_av_cb.audio_open_cnt == 1) - policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; - bta_sys_set_policy(BTA_ID_AV, policy, p_scb->PeerAddress()); + bta_av_cb.audio_open_cnt == 1) { + set_policy |= HCI_ENABLE_MASTER_SLAVE_SWITCH; + } + bta_sys_set_policy(BTA_ID_AV, set_policy, p_scb->PeerAddress()); /* in case that we received suspend_ind, we may need to call co_stop here */ if (p_scb->co_started) { diff --git a/bta/av/bta_av_main.cc b/bta/av/bta_av_main.cc index da04d7148..b1cde50f5 100644 --- a/bta/av/bta_av_main.cc +++ b/bta/av/bta_av_main.cc @@ -835,13 +835,14 @@ void bta_av_restore_switch(void) { tBTA_AV_CB* p_cb = &bta_av_cb; int i; uint8_t mask; + uint8_t set_policy = (HCI_ENABLE_SNIFF_MODE | HCI_ENABLE_MASTER_SLAVE_SWITCH); APPL_TRACE_DEBUG("%s: reg_audio: 0x%x", __func__, bta_av_cb.reg_audio); for (i = 0; i < BTA_AV_NUM_STRS; i++) { mask = BTA_AV_HNDL_TO_MSK(i); if (p_cb->conn_audio == mask) { if (p_cb->p_scb[i]) { - bta_sys_set_policy(BTA_ID_AV, HCI_ENABLE_MASTER_SLAVE_SWITCH, + bta_sys_set_policy(BTA_ID_AV, set_policy, p_cb->p_scb[i]->PeerAddress()); } break; @@ -865,6 +866,7 @@ static void bta_av_sys_rs_cback(UNUSED_ATTR tBTA_SYS_CONN_STATUS status, tBTA_AV_SCB* p_scb = NULL; uint8_t cur_role; uint8_t peer_idx = 0; + uint8_t set_policy = (HCI_ENABLE_SNIFF_MODE | HCI_ENABLE_MASTER_SLAVE_SWITCH); APPL_TRACE_DEBUG( "%s: peer %s new_role:%d hci_status:0x%x bta_av_cb.rs_idx:%d", __func__, @@ -884,9 +886,7 @@ static void bta_av_sys_rs_cback(UNUSED_ATTR tBTA_SYS_CONN_STATUS status, /* if ((id != BTM_ROLE_MASTER) && (app_id != HCI_SUCCESS)) { - bta_sys_set_policy(BTA_ID_AV, - (HCI_ENABLE_MASTER_SLAVE_SWITCH|HCI_ENABLE_SNIFF_MODE), - p_scb->PeerAddress()); + bta_sys_set_policy(BTA_ID_AV, set_policy, p_scb->PeerAddress()); } */ p_buf->hdr.event = BTA_AV_ROLE_CHANGE_EVT; @@ -903,7 +903,7 @@ static void bta_av_sys_rs_cback(UNUSED_ATTR tBTA_SYS_CONN_STATUS status, if ((HCI_SUCCESS != app_id) && (BTM_GetRole(peer_addr, &cur_role) == BTM_SUCCESS) && (cur_role == BTM_ROLE_SLAVE)) { - bta_sys_set_policy(BTA_ID_AV, HCI_ENABLE_MASTER_SLAVE_SWITCH, peer_addr); + bta_sys_set_policy(BTA_ID_AV, set_policy, peer_addr); } /* if BTA_AvOpen() was called for other device, which caused the role switch diff --git a/bta/dm/bta_dm_cfg.cc b/bta/dm/bta_dm_cfg.cc index 9b7806c77..91053a19b 100644 --- a/bta/dm/bta_dm_cfg.cc +++ b/bta/dm/bta_dm_cfg.cc @@ -236,7 +236,7 @@ tBTA_DM_PM_TYPE_QUALIFIER tBTA_DM_PM_SPEC bta_dm_pm_spec[BTA_DM_NUM_PM_SPEC] = { /* AV : 4 */ {(BTA_DM_PM_SNIFF), /* allow sniff */ #if (BTM_SSR_INCLUDED == TRUE) - (BTA_DM_PM_SSR2), /* the SSR entry */ + (BTA_DM_PM_SSR0), /* the SSR entry */ #endif { {{BTA_DM_PM_SNIFF_A2DP_IDX, 7000}, diff --git a/bta/sys/bta_sys.h b/bta/sys/bta_sys.h index 9a3dec0e6..84f22f294 100644 --- a/bta/sys/bta_sys.h +++ b/bta/sys/bta_sys.h @@ -266,11 +266,11 @@ extern void bta_sys_chg_ssr_config(uint8_t id, uint8_t app_id, #endif extern void bta_sys_role_chg_register(tBTA_SYS_CONN_CBACK* p_cback); -extern void bta_sys_notify_role_chg(const RawAddress& p_bda, uint8_t new_role, - uint8_t hci_status); +extern void bta_sys_notify_role_chg(const RawAddress& peer_addr, + uint8_t new_role, uint8_t hci_status); extern void bta_sys_collision_register(uint8_t bta_id, tBTA_SYS_CONN_CBACK* p_cback); -extern void bta_sys_notify_collision(const RawAddress& p_bda); +extern void bta_sys_notify_collision(const RawAddress& peer_addr); #if (BTA_EIR_CANNED_UUID_LIST != TRUE) extern void bta_sys_eir_register(tBTA_SYS_EIR_CBACK* p_cback); diff --git a/bta/sys/bta_sys_conn.cc b/bta/sys/bta_sys_conn.cc index 3c3bc63c8..ef9649b79 100644 --- a/bta/sys/bta_sys_conn.cc +++ b/bta/sys/bta_sys_conn.cc @@ -97,12 +97,12 @@ void bta_sys_ssr_cfg_register(tBTA_SYS_SSR_CFG_CBACK* p_cback) { * Returns void * ******************************************************************************/ -void bta_sys_notify_role_chg(const RawAddress& p_bda, uint8_t new_role, +void bta_sys_notify_role_chg(const RawAddress& peer_addr, uint8_t new_role, uint8_t hci_status) { APPL_TRACE_DEBUG("%s: peer %s new_role:%d hci_status:0x%x", __func__, - p_bda.ToString().c_str(), new_role, hci_status); + peer_addr.ToString().c_str(), new_role, hci_status); if (bta_sys_cb.p_role_cb) { - bta_sys_cb.p_role_cb(BTA_SYS_ROLE_CHANGE, new_role, hci_status, p_bda); + bta_sys_cb.p_role_cb(BTA_SYS_ROLE_CHANGE, new_role, hci_status, peer_addr); } } @@ -139,13 +139,13 @@ void bta_sys_collision_register(uint8_t bta_id, tBTA_SYS_CONN_CBACK* p_cback) { * Returns void * ******************************************************************************/ -void bta_sys_notify_collision(const RawAddress& p_bda) { +void bta_sys_notify_collision(const RawAddress& peer_addr) { uint8_t index; for (index = 0; index < MAX_COLLISION_REG; index++) { if ((bta_sys_cb.colli_reg.id[index] != 0) && (bta_sys_cb.colli_reg.p_coll_cback[index] != NULL)) { - bta_sys_cb.colli_reg.p_coll_cback[index](0, BTA_ID_SYS, 0, p_bda); + bta_sys_cb.colli_reg.p_coll_cback[index](0, BTA_ID_SYS, 0, peer_addr); } } } @@ -368,6 +368,8 @@ void bta_sys_chg_ssr_config(uint8_t id, uint8_t app_id, uint16_t max_latency, ******************************************************************************/ void bta_sys_set_policy(uint8_t id, uint8_t policy, const RawAddress& peer_addr) { + APPL_TRACE_DEBUG("%s: peer %s id:%d policy:0x%x", __func__, + peer_addr.ToString().c_str(), id, policy); if (bta_sys_cb.p_policy_cb) { bta_sys_cb.p_policy_cb(BTA_SYS_PLCY_SET, id, policy, peer_addr); } @@ -385,6 +387,8 @@ void bta_sys_set_policy(uint8_t id, uint8_t policy, ******************************************************************************/ void bta_sys_clear_policy(uint8_t id, uint8_t policy, const RawAddress& peer_addr) { + APPL_TRACE_DEBUG("%s: peer %s id:%d policy:0x%x", __func__, + peer_addr.ToString().c_str(), id, policy); if (bta_sys_cb.p_policy_cb) { bta_sys_cb.p_policy_cb(BTA_SYS_PLCY_CLR, id, policy, peer_addr); } @@ -401,6 +405,7 @@ void bta_sys_clear_policy(uint8_t id, uint8_t policy, * ******************************************************************************/ void bta_sys_set_default_policy(uint8_t id, uint8_t policy) { + APPL_TRACE_DEBUG("%s: id:%d policy:0x%x", __func__, id, policy); if (bta_sys_cb.p_policy_cb) { bta_sys_cb.p_policy_cb(BTA_SYS_PLCY_DEF_SET, id, policy, RawAddress::kEmpty); @@ -418,6 +423,7 @@ void bta_sys_set_default_policy(uint8_t id, uint8_t policy) { * ******************************************************************************/ void bta_sys_clear_default_policy(uint8_t id, uint8_t policy) { + APPL_TRACE_DEBUG("%s: id:%d policy:0x%x", __func__, id, policy); if (bta_sys_cb.p_policy_cb) { bta_sys_cb.p_policy_cb(BTA_SYS_PLCY_DEF_CLR, id, policy, RawAddress::kEmpty); diff --git a/internal_include/bt_trace.h b/internal_include/bt_trace.h index 9ab9eb07a..97c761bca 100644 --- a/internal_include/bt_trace.h +++ b/internal_include/bt_trace.h @@ -741,6 +741,18 @@ std::string loghex(T x) { } /** + * Obtains the string representation of a boolean value. + * + * @param value the boolean value to use + * @return the string representation of the boolean value: "true" or "false" + */ +inline std::string logbool(bool value) { + std::stringstream tmp; + tmp << std::boolalpha << value; + return tmp.str(); +} + +/** * Append a field name to a string. * * The field names are added to the string with "|" in between. -- 2.11.0