LinuxLists.cc - [PATCH v10 01/10] Bluetooth: Enumerate local supported codec and cache details

2021-06-30 08:06:22

Subject: [PATCH v10 01/10] Bluetooth: Enumerate local supported codec and cache details

Move reading of supported local codecs into a separate init function,
query codecs capabilities and cache the data

Signed-off-by: Kiran K <[email protected]>
Signed-off-by: Chethan T N <[email protected]>
Signed-off-by: Srivatsa Ravishankar <[email protected]>
---
* changes in v10:
- define Kconfig to control offload feature at build time
- fix review comments

* changes in v9:
- use shortname vnd instead of ven

* changes in v8:
- add comments
- split __u8 codec_id[5] into {__u8 id; __le16 cid, vid }
- address review comment related codec caps structure

* changes in v7:
- keep codec enumeration call in hci_init instead of having a separate
function
- Remove unused bitmasks defined for LE transports

* changes in v6:
- fix compiler warning reported for ARCH=arc

* changes in v5:
- fix review comments
- move code used to read standard/vendor codecs caps into single function

* changes in v4:
- convert reading of codecs and codecs caps calls from async to sync

* changes in v3
move codec enumeration into a new init function

* changes in v2
add skb length check before accessing data

include/net/bluetooth/hci.h | 41 ++++++
include/net/bluetooth/hci_core.h | 23 ++++
net/bluetooth/Kconfig | 11 ++
net/bluetooth/hci_core.c | 207 ++++++++++++++++++++++++++++++-
4 files changed, 278 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b80415011dcd..f76849c8eafd 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1307,6 +1307,28 @@ struct hci_rp_read_data_block_size {
} __packed;

#define HCI_OP_READ_LOCAL_CODECS 0x100b
+struct hci_std_codecs {
+ __u8 num;
+ __u8 codec[];
+} __packed;
+
+struct hci_vnd_codec {
+ /* company id */
+ __le16 cid;
+ /* vendor codec id */
+ __le16 vid;
+} __packed;
+
+struct hci_vnd_codecs {
+ __u8 num;
+ struct hci_vnd_codec codec[];
+} __packed;
+
+struct hci_rp_read_local_supported_codecs {
+ __u8 status;
+ struct hci_std_codecs std_codecs;
+ struct hci_vnd_codecs vnd_codecs;
+} __packed;

#define HCI_OP_READ_LOCAL_PAIRING_OPTS 0x100c
struct hci_rp_read_local_pairing_opts {
@@ -1315,6 +1337,25 @@ struct hci_rp_read_local_pairing_opts {
__u8 max_key_size;
} __packed;

+#define HCI_OP_READ_LOCAL_CODEC_CAPS 0x100e
+struct hci_op_read_local_codec_caps {
+ __u8 id;
+ __le16 cid;
+ __le16 vid;
+ __u8 transport;
+ __u8 direction;
+} __packed;
+
+struct hci_codec_caps {
+ __u8 len;
+ __u8 data[];
+} __packed;
+
+struct hci_rp_read_local_codec_caps {
+ __u8 status;
+ __u8 num_caps;
+} __packed;
+
#define HCI_OP_READ_PAGE_SCAN_ACTIVITY 0x0c1b
struct hci_rp_read_page_scan_activity {
__u8 status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a53e94459ecd..7b8d603358b9 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -131,6 +131,19 @@ struct bdaddr_list {
u8 bdaddr_type;
};

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+struct codec_list {
+ struct list_head list;
+ u8 id;
+ __le16 cid;
+ __le16 vid;
+ u8 transport;
+ u8 num_caps;
+ u32 len;
+ struct hci_codec_caps caps[];
+};
+#endif
+
struct bdaddr_list_with_irk {
struct list_head list;
bdaddr_t bdaddr;
@@ -535,6 +548,9 @@ struct hci_dev {
struct list_head pend_le_conns;
struct list_head pend_le_reports;
struct list_head blocked_keys;
+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+ struct list_head local_codecs;
+#endif

struct hci_dev_stats stat;

@@ -1849,4 +1865,11 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
#define SCO_AIRMODE_CVSD 0x0000
#define SCO_AIRMODE_TRANSP 0x0003

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+#define LOCAL_CODEC_ACL_MASK BIT(0)
+#define LOCAL_CODEC_SCO_MASK BIT(1)
+
+#define TRANSPORT_TYPE_MAX 0x04
+#endif
+
#endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
index e0ab4cd7afc3..1069623f36c4 100644
--- a/net/bluetooth/Kconfig
+++ b/net/bluetooth/Kconfig
@@ -92,6 +92,17 @@ config BT_LEDS
This option selects a few LED triggers for different
Bluetooth events.

+config BT_OFFLOAD_CODECS
+ bool "Enable offload codecs"
+ depends on BT && BT_BREDR
+ default n
+ help
+ This option enables offload codecs if controller supports.
+ When this option is enabled, user space audio modules can
+ query offload codecs and can set the codec to be used for
+ specific use case.
+
+
config BT_MSFTEXT
bool "Enable Microsoft extensions"
depends on BT
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2560ed2f144d..2657d84e8240 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -838,10 +838,6 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
if (hdev->commands[22] & 0x04)
hci_set_event_mask_page_2(req);

- /* Read local codec list if the HCI command is supported */
- if (hdev->commands[29] & 0x20)
- hci_req_add(req, HCI_OP_READ_LOCAL_CODECS, 0, NULL);
-
/* Read local pairing options if the HCI command is supported */
if (hdev->commands[41] & 0x08)
hci_req_add(req, HCI_OP_READ_LOCAL_PAIRING_OPTS, 0, NULL);
@@ -907,6 +903,197 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
return 0;
}

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+static int hci_codec_list_add(struct list_head *list,
+ struct hci_op_read_local_codec_caps *sent,
+ struct hci_rp_read_local_codec_caps *rp,
+ void *caps,
+ __u32 len)
+{
+ struct codec_list *entry;
+
+ entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->id = sent->id;
+ if (sent->id == 0xFF) {
+ entry->cid = __le16_to_cpu(sent->cid);
+ entry->vid = __le16_to_cpu(sent->vid);
+ }
+ entry->transport = sent->transport;
+ entry->len = len;
+ entry->num_caps = rp->num_caps;
+ if (rp->num_caps)
+ memcpy(entry->caps, caps, len);
+ list_add(&entry->list, list);
+
+ return 0;
+}
+
+static void hci_codec_list_clear(struct list_head *codec_list)
+{
+ struct codec_list *c, *n;
+
+ list_for_each_entry_safe(c, n, codec_list, list) {
+ list_del(&c->list);
+ kfree(c);
+ }
+}
+
+static void hci_read_codec_capabilities(struct hci_dev *hdev, void *codec_id,
+ __u8 transport, bool is_vnd_codec)
+{
+ struct hci_op_read_local_codec_caps cmd;
+ __u8 i;
+
+ memset(&cmd, 0, sizeof(cmd));
+
+ if (is_vnd_codec) {
+ struct hci_vnd_codec *vnd_codec;
+
+ vnd_codec = codec_id;
+ cmd.id = 0xFF;
+ cmd.cid = vnd_codec->cid;
+ cmd.vid = vnd_codec->vid;
+ } else {
+ cmd.id = *(__u8 *)codec_id;
+ }
+
+ cmd.direction = 0x00;
+
+ for (i = 0; i < TRANSPORT_TYPE_MAX; i++) {
+ if (transport & BIT(i)) {
+ struct hci_rp_read_local_codec_caps *rp;
+ struct hci_codec_caps *caps;
+ struct sk_buff *skb;
+ __u8 j;
+ __u32 len;
+
+ cmd.transport = i;
+ skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
+ sizeof(cmd), &cmd,
+ HCI_CMD_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Failed to read codec capabilities (%ld)",
+ PTR_ERR(skb));
+ continue;
+ }
+
+ if (skb->len < sizeof(*rp))
+ goto error;
+
+ rp = (void *)skb->data;
+
+ if (rp->status)
+ goto error;
+
+ if (!rp->num_caps) {
+ len = 0;
+ /* this codec doesn't have capabilities */
+ goto skip_caps_parse;
+ }
+
+ skb_pull(skb, sizeof(*rp));
+
+ for (j = 0, len = 0; j < rp->num_caps; j++) {
+ caps = (void *)skb->data;
+ if (skb->len < sizeof(*caps))
+ goto error;
+ if (skb->len < caps->len)
+ goto error;
+ len += sizeof(caps->len) + caps->len;
+ skb_pull(skb, sizeof(caps->len) + caps->len);
+ }
+
+skip_caps_parse:
+ hci_dev_lock(hdev);
+ hci_codec_list_add(&hdev->local_codecs, &cmd, rp,
+ (__u8 *)rp + sizeof(*rp), len);
+ hci_dev_unlock(hdev);
+error:
+ kfree_skb(skb);
+ }
+ }
+}
+
+static void hci_codec_list_parse(struct hci_dev *hdev, __u8 num_codecs,
+ void *codec_list, bool is_vnd_codec)
+{
+ __u8 i;
+
+ for (i = 0; i < num_codecs; i++) {
+ if (!is_vnd_codec) {
+ struct hci_std_codecs *codecs = codec_list;
+
+ hci_read_codec_capabilities(hdev, &codecs->codec[i],
+ LOCAL_CODEC_ACL_MASK,
+ is_vnd_codec);
+ } else {
+ struct hci_vnd_codecs *codecs = codec_list;
+
+ hci_read_codec_capabilities(hdev, &codecs->codec[i],
+ LOCAL_CODEC_ACL_MASK,
+ is_vnd_codec);
+ }
+ }
+}
+
+static void hci_read_supported_codecs(struct hci_dev *hdev)
+{
+ struct sk_buff *skb;
+ struct hci_rp_read_local_supported_codecs *rp;
+ struct hci_std_codecs *std_codecs;
+ struct hci_vnd_codecs *vnd_codecs;
+
+ skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODECS, 0, NULL,
+ HCI_CMD_TIMEOUT);
+
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
+ PTR_ERR(skb));
+ return;
+ }
+
+ if (skb->len < sizeof(*rp))
+ goto error;
+
+ rp = (void *)skb->data;
+
+ if (rp->status)
+ goto error;
+
+ skb_pull(skb, sizeof(rp->status));
+
+ std_codecs = (void *)skb->data;
+
+ /* validate codecs length before accessing */
+ if (skb->len < flex_array_size(std_codecs, codec, std_codecs->num)
+ + sizeof(std_codecs->num))
+ goto error;
+
+ /* enumerate codec capabilities of standard codecs */
+ hci_codec_list_parse(hdev, std_codecs->num, std_codecs, false);
+
+ skb_pull(skb, flex_array_size(std_codecs, codec, std_codecs->num)
+ + sizeof(std_codecs->num));
+
+ vnd_codecs = (void *)skb->data;
+
+ /* validate vendor codecs length before accessing */
+ if (skb->len <
+ flex_array_size(vnd_codecs, codec, vnd_codecs->num)
+ + sizeof(vnd_codecs->num))
+ goto error;
+
+ /* enumerate vendor codec capabilities */
+ hci_codec_list_parse(hdev, vnd_codecs->num, vnd_codecs, true);
+
+error:
+ kfree_skb(skb);
+}
+#endif
+
static int __hci_init(struct hci_dev *hdev)
{
int err;
@@ -937,6 +1124,12 @@ static int __hci_init(struct hci_dev *hdev)
if (err < 0)
return err;

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+ /* Read local codec list if the HCI command is supported */
+ if (hdev->commands[29] & 0x20)
+ hci_read_supported_codecs(hdev);
+#endif
+
/* This function is only called when the controller is actually in
* configured state. When the controller is marked as unconfigured,
* this initialization procedure is not run.
@@ -1841,6 +2034,9 @@ int hci_dev_do_close(struct hci_dev *hdev)
memset(hdev->eir, 0, sizeof(hdev->eir));
memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
bacpy(&hdev->random_addr, BDADDR_ANY);
+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+ hci_codec_list_clear(&hdev->local_codecs);
+#endif

hci_req_sync_unlock(hdev);

@@ -3843,6 +4039,9 @@ struct hci_dev *hci_alloc_dev(void)
INIT_LIST_HEAD(&hdev->adv_instances);
INIT_LIST_HEAD(&hdev->blocked_keys);

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+ INIT_LIST_HEAD(&hdev->local_codecs);
+#endif
INIT_WORK(&hdev->rx_work, hci_rx_work);
INIT_WORK(&hdev->cmd_work, hci_cmd_work);
INIT_WORK(&hdev->tx_work, hci_tx_work);
--
2.17.1

2021-06-30 08:08:46

by Kiran K

[permalink] [raw]

Subject: [PATCH v10 07/10] Bluetooth: btintel: Define a callback to fetch codec config data

Define callback function to get codec config data. In HFP offload
usecase, controllers need to be set codec details before opening SCO.
This callback function is used to fetch vendor specific codec config
data.

Signed-off-by: Kiran K <[email protected]>
Reviewed-by: Chethan T N <[email protected]>
Reviewed-by: Srivatsa Ravishankar <[email protected]>
---
* changes in v10:
- new patch due to refactoring

drivers/bluetooth/btintel.c | 49 ++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index cdb098235e88..df2e84860c91 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1273,6 +1273,51 @@ int btintel_set_debug_features(struct hci_dev *hdev,
EXPORT_SYMBOL_GPL(btintel_set_debug_features);

#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+static int btintel_get_codec_config_data(struct hci_dev *hdev,
+ __u8 link, struct bt_codec *codec,
+ __u8 *ven_len, __u8 **ven_data)
+{
+ int err = 0;
+
+ if (!ven_data || !ven_len)
+ return -EINVAL;
+
+ *ven_len = 0;
+ *ven_data = NULL;
+
+ if (link != SCO_LINK && link != ESCO_LINK)
+ return -EINVAL;
+
+ *ven_data = kmalloc(sizeof(__u8), GFP_KERNEL);
+ if (!ven_data) {
+ err = -ENOMEM;
+ goto error;
+ }
+
+ /* supports only CVSD and mSBC offload codecs */
+ switch (codec->id) {
+ case 0x02:
+ **ven_data = 0x00;
+ break;
+ case 0x05:
+ **ven_data = 0x01;
+ break;
+ default:
+ err = -EINVAL;
+ goto error;
+ }
+ /* codec and its capabilities are pre-defined to ids
+ * preset id = 0x00 represents CVSD codec with sampling rate 8K
+ * preset id = 0x01 represents mSBC codec with sampling rate 16K
+ */
+ *ven_len = sizeof(__u8);
+ return err;
+
+error:
+ kfree(*ven_data);
+ return err;
+}
+
static int btintel_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
{
/* Intel uses 1 as data path id for all the usecases */
@@ -1305,8 +1350,10 @@ int btintel_configure_offload_usecases(struct hci_dev *hdev)
goto error;
}

- if (usecases->preset[0] & 0x03)
+ if (usecases->preset[0] & 0x03) {
hdev->get_data_path_id = btintel_get_data_path_id;
+ hdev->get_codec_config_data = btintel_get_codec_config_data;
+ }
error:
kfree_skb(skb);
return err;
--
2.17.1

2021-06-30 08:08:46

by Kiran K

[permalink] [raw]

Subject: [PATCH v10 10/10] Bluetooth: Add offload feature under experimental flag

Allow user level process to enable / disable codec offload
feature through mgmt interface. By default offload codec feature
is disabled.

Signed-off-by: Kiran K <[email protected]>
Reviewed-by: Chethan T N <[email protected]>
Reviewed-by: Srivatsa Ravishankar <[email protected]>
---
* changes in v10:
- new patch added to place offload codec feature under experimental flag

include/net/bluetooth/hci.h | 4 ++
net/bluetooth/mgmt.c | 106 +++++++++++++++++++++++++++++++++++-
net/bluetooth/sco.c | 10 ++++
3 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e20318854900..5ca98d9f64dd 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -331,6 +331,10 @@ enum {
HCI_CMD_PENDING,
HCI_FORCE_NO_MITM,

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+ HCI_OFFLOAD_CODECS_ENABLED,
+#endif
+
__HCI_NUM_FLAGS,
};

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3663f880df11..d7be85eb52e7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3791,6 +3791,14 @@ static const u8 debug_uuid[16] = {
};
#endif

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+/* a6695ace-ee7f-4fb9-881a-5fac66c629af */
+static const u8 offload_codecs_uuid[16] = {
+ 0xaf, 0x29, 0xc6, 0x66, 0xac, 0x5f, 0x1a, 0x88,
+ 0xb9, 0x4f, 0x7f, 0xee, 0xce, 0x5a, 0x69, 0xa6,
+};
+#endif
+
/* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
static const u8 simult_central_periph_uuid[16] = {
0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
@@ -3806,7 +3814,7 @@ static const u8 rpa_resolution_uuid[16] = {
static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
void *data, u16 data_len)
{
- char buf[62]; /* Enough space for 3 features */
+ char buf[82]; /* Enough space for 4 features: 2 + 20 * 4 */
struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
u16 idx = 0;
u32 flags;
@@ -3850,6 +3858,28 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
idx++;
}

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+ if (hdev) {
+ if (hdev->get_data_path_id) {
+ /* BIT(0): indicating if offload codecs are
+ * supported by controller.
+ */
+ flags = BIT(0);
+
+ /* BIT(1): indicating if codec offload feature
+ * is enabled.
+ */
+ if (hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED))
+ flags |= BIT(1);
+ } else {
+ flags = 0;
+ }
+ memcpy(rp->features[idx].uuid, offload_codecs_uuid, 16);
+ rp->features[idx].flags = cpu_to_le32(flags);
+ idx++;
+ }
+#endif
+
rp->feature_count = cpu_to_le16(idx);

/* After reading the experimental features information, enable
@@ -3892,6 +3922,23 @@ static int exp_debug_feature_changed(bool enabled, struct sock *skip)
}
#endif

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+static int exp_offload_codec_feature_changed(bool enabled, struct sock *skip)
+{
+ struct mgmt_ev_exp_feature_changed ev;
+
+ BT_INFO("enabled %d", enabled);
+
+ memset(&ev, 0, sizeof(ev));
+ memcpy(ev.uuid, offload_codecs_uuid, 16);
+ ev.flags = cpu_to_le32(enabled ? BIT(0) : 0);
+
+ return mgmt_limited_event(MGMT_EV_EXP_FEATURE_CHANGED, NULL,
+ &ev, sizeof(ev),
+ HCI_MGMT_EXP_FEATURE_EVENTS, skip);
+}
+#endif
+
static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
void *data, u16 data_len)
{
@@ -4038,6 +4085,63 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
return err;
}

+#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
+ if (!memcmp(cp->uuid, offload_codecs_uuid, 16)) {
+ bool val, changed;
+ int err;
+
+ /* Command requires to use a valid controller index */
+ if (!hdev)
+ return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+ MGMT_OP_SET_EXP_FEATURE,
+ MGMT_STATUS_INVALID_INDEX);
+
+ /* Parameters are limited to a single octet */
+ if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_EXP_FEATURE,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ /* Only boolean on/off is supported */
+ if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_EXP_FEATURE,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ val = !!cp->param[0];
+ changed = (val != hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED));
+
+ if (!hdev->get_data_path_id) {
+ bt_dev_info(hdev, "offload codecs not supported");
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_EXP_FEATURE,
+ MGMT_STATUS_NOT_SUPPORTED);
+ }
+
+ if (changed) {
+ if (val)
+ hci_dev_set_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED);
+ else
+ hci_dev_clear_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED);
+ }
+
+ bt_dev_info(hdev, "offload codecs enable %d changed %d",
+ val, changed);
+
+ memcpy(rp.uuid, offload_codecs_uuid, 16);
+ rp.flags = cpu_to_le32(val ? BIT(0) : 0);
+ hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
+ err = mgmt_cmd_complete(sk, hdev->id,
+ MGMT_OP_SET_EXP_FEATURE, 0,
+ &rp, sizeof(rp));
+
+ if (changed)
+ exp_offload_codec_feature_changed(val, sk);
+
+ return err;
+ }
+#endif
+
return mgmt_cmd_status(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
MGMT_OP_SET_EXP_FEATURE,
MGMT_STATUS_NOT_SUPPORTED);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index b44d56eb936a..bc033464af43 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -964,6 +964,11 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
break;
}

+ if (!hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED)) {
+ err = -EOPNOTSUPP;
+ break;
+ }
+
if (!hdev->get_codec_config_data) {
err = -EOPNOTSUPP;
break;
@@ -1163,6 +1168,11 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
break;
}

+ if (!hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED)) {
+ err = -EOPNOTSUPP;
+ break;
+ }
+
if (!hdev->get_data_path_id) {
err = -EOPNOTSUPP;
break;
--
2.17.1

2021-06-30 08:10:22

by Kiran K

[permalink] [raw]

Subject: [PATCH v10 08/10] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command

< HCI Command: Enhanced Setup Synchronous Connection (0x01|0x003d) plen 59
Handle: 256
Transmit bandwidth: 8000
Receive bandwidth: 8000
Max latency: 13
Packet type: 0x0380
3-EV3 may not be used
2-EV5 may not be used
3-EV5 may not be used
Retransmission effort: Optimize for link quality (0x02)
> HCI Event: Command Status (0x0f) plen 4
Enhanced Setup Synchronous Connection (0x01|0x003d) ncmd 1
Status: Success (0x00)
> HCI Event: Synchronous Connect Complete (0x2c) plen 17
Status: Success (0x00)
Handle: 257
Address: CC:98:8B:92:04:FD (SONY Visual Products Inc.)
Link type: eSCO (0x02)
Transmission interval: 0x0c
Retransmission window: 0x06
RX packet length: 60
TX packet length: 60
Air mode: Transparent (0x03)

Signed-off-by: Kiran K <[email protected]>
Reviewed-by: Chethan T N <[email protected]>
Reviewed-by: Srivatsa Ravishankar <[email protected]>
---
* changes in v10:
- Fix typos and remove unwanted chunks
* changes in v9:
- Fix review comments, use bt_dev_dbg instead of BT_DBG

include/net/bluetooth/bluetooth.h | 3 +-
include/net/bluetooth/hci.h | 34 ++++++++++
include/net/bluetooth/hci_core.h | 7 +-
net/bluetooth/hci_conn.c | 106 ++++++++++++++++++++++++++++--
net/bluetooth/hci_event.c | 48 +++++++++++++-
net/bluetooth/sco.c | 13 +++-
6 files changed, 201 insertions(+), 10 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 0e8802d09068..af2809121571 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -173,7 +173,8 @@ struct bt_codecs {
struct bt_codec codecs[];
} __packed;

-#define CODING_FORMAT_CVSD 0x02
+#define CODING_FORMAT_CVSD 0x02
+#define CODING_FORMAT_TRANSPARENT 0x03

__printf(1, 2)
void bt_info(const char *fmt, ...);
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ae384b7bf000..e20318854900 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -871,6 +871,40 @@ struct hci_cp_logical_link_cancel {
__u8 flow_spec_id;
} __packed;

+#define HCI_OP_ENHANCED_SETUP_SYNC_CONN 0x043D
+struct hci_coding_format {
+ __u8 id;
+ __le16 cid;
+ __le16 vid;
+} __packed;
+
+struct hci_cp_enhanced_setup_sync_conn {
+ __le16 handle;
+ __le32 tx_bandwidth;
+ __le32 rx_bandwidth;
+ struct hci_coding_format tx_coding_format;
+ struct hci_coding_format rx_coding_format;
+ __le16 tx_codec_frame_size;
+ __le16 rx_codec_frame_size;
+ __le32 in_bandwidth;
+ __le32 out_bandwidth;
+ struct hci_coding_format in_coding_format;
+ struct hci_coding_format out_coding_format;
+ __le16 in_coded_data_size;
+ __le16 out_coded_data_size;
+ __u8 in_pcm_data_format;
+ __u8 out_pcm_data_format;
+ __u8 in_pcm_sample_payload_msb_pos;
+ __u8 out_pcm_sample_payload_msb_pos;
+ __u8 in_data_path;
+ __u8 out_data_path;
+ __u8 in_transport_unit_size;
+ __u8 out_transport_unit_size;
+ __le16 max_latency;
+ __le16 pkt_type;
+ __u8 retrans_effort;
+} __packed;
+
struct hci_rp_logical_link_cancel {
__u8 status;
__u8 phy_handle;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebf42a3ac6cd..80ed81fb1883 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -719,6 +719,7 @@ struct hci_conn {
struct amp_mgr *amp_mgr;

struct hci_conn *link;
+ struct bt_codec codec;

void (*connect_cfm_cb) (struct hci_conn *conn, u8 status);
void (*security_cfm_cb) (struct hci_conn *conn, u8 status);
@@ -1101,6 +1102,7 @@ static inline struct hci_conn *hci_lookup_le_connect(struct hci_dev *hdev)

int hci_disconnect(struct hci_conn *conn, __u8 reason);
bool hci_setup_sync(struct hci_conn *conn, __u16 handle);
+bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle);
void hci_sco_setup(struct hci_conn *conn, __u8 status);

struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
@@ -1125,7 +1127,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
u8 sec_level, u8 auth_type,
enum conn_reasons conn_reason);
struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u16 setting);
+ __u16 setting, struct bt_codec *codec);
int hci_conn_check_link_mode(struct hci_conn *conn);
int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type,
@@ -1446,6 +1448,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
/* Use LL Privacy based address resolution if supported */
#define use_ll_privacy(dev) ((dev)->le_features[0] & HCI_LE_LL_PRIVACY)

+/* Use enhanced synchronous connection if command is supported */
+#define use_enhanced_sco_conn(dev) ((dev)->commands[29] & 0x08)
+
/* Use ext scanning if set ext scan param and ext scan enable is supported */
#define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
((dev)->commands[37] & 0x40))
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2b5059a56cda..d4a08b344ad0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -307,6 +307,96 @@ static bool find_next_esco_param(struct hci_conn *conn,
return conn->attempt <= size;
}

+bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
+{
+ struct hci_dev *hdev = conn->hdev;
+ struct hci_cp_enhanced_setup_sync_conn cp;
+ const struct sco_param *param;
+
+ bt_dev_dbg(hdev, "hcon %p", conn);
+
+ conn->state = BT_CONNECT;
+ conn->out = true;
+
+ conn->attempt++;
+
+ memset(&cp, 0x00, sizeof(cp));
+
+ cp.handle = cpu_to_le16(handle);
+
+ cp.tx_bandwidth = cpu_to_le32(0x00001f40);
+ cp.rx_bandwidth = cpu_to_le32(0x00001f40);
+
+ switch (conn->codec.id) {
+ case CODING_FORMAT_TRANSPARENT:
+ if (!find_next_esco_param(conn, esco_param_msbc,
+ ARRAY_SIZE(esco_param_msbc)))
+ return false;
+ param = &esco_param_msbc[conn->attempt - 1];
+ cp.tx_coding_format.id = 0x03;
+ cp.rx_coding_format.id = 0x03;
+ cp.tx_codec_frame_size = __cpu_to_le16(60);
+ cp.rx_codec_frame_size = __cpu_to_le16(60);
+ cp.in_bandwidth = __cpu_to_le32(0x1f40);
+ cp.out_bandwidth = __cpu_to_le32(0x1f40);
+ cp.in_coding_format.id = 0x03;
+ cp.out_coding_format.id = 0x03;
+ cp.in_coded_data_size = __cpu_to_le16(16);
+ cp.out_coded_data_size = __cpu_to_le16(16);
+ cp.in_pcm_data_format = 2;
+ cp.out_pcm_data_format = 2;
+ cp.in_pcm_sample_payload_msb_pos = 0;
+ cp.out_pcm_sample_payload_msb_pos = 0;
+ cp.in_data_path = conn->codec.data_path;
+ cp.out_data_path = conn->codec.data_path;
+ cp.in_transport_unit_size = 1;
+ cp.out_transport_unit_size = 1;
+ break;
+
+ case CODING_FORMAT_CVSD:
+ if (lmp_esco_capable(conn->link)) {
+ if (!find_next_esco_param(conn, esco_param_cvsd,
+ ARRAY_SIZE(esco_param_cvsd)))
+ return false;
+ param = &esco_param_cvsd[conn->attempt - 1];
+ } else {
+ if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
+ return false;
+ param = &sco_param_cvsd[conn->attempt - 1];
+ }
+ cp.tx_coding_format.id = 2;
+ cp.rx_coding_format.id = 2;
+ cp.tx_codec_frame_size = __cpu_to_le16(60);
+ cp.rx_codec_frame_size = __cpu_to_le16(60);
+ cp.in_bandwidth = __cpu_to_le32(16000);
+ cp.out_bandwidth = __cpu_to_le32(16000);
+ cp.in_coding_format.id = 4;
+ cp.out_coding_format.id = 4;
+ cp.in_coded_data_size = __cpu_to_le16(16);
+ cp.out_coded_data_size = __cpu_to_le16(16);
+ cp.in_pcm_data_format = 2;
+ cp.out_pcm_data_format = 2;
+ cp.in_pcm_sample_payload_msb_pos = 0;
+ cp.out_pcm_sample_payload_msb_pos = 0;
+ cp.in_data_path = conn->codec.data_path;
+ cp.out_data_path = conn->codec.data_path;
+ cp.in_transport_unit_size = 16;
+ cp.out_transport_unit_size = 16;
+ break;
+ default:
+ return false;
+ }
+
+ cp.retrans_effort = param->retrans_effort;
+ cp.pkt_type = __cpu_to_le16(param->pkt_type);
+ cp.max_latency = __cpu_to_le16(param->max_latency);
+
+ if (hci_send_cmd(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN, sizeof(cp), &cp) < 0)
+ return false;
+
+ return true;
+}
+
bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
{
struct hci_dev *hdev = conn->hdev;
@@ -424,10 +514,14 @@ void hci_sco_setup(struct hci_conn *conn, __u8 status)
BT_DBG("hcon %p", conn);

if (!status) {
- if (lmp_esco_capable(conn->hdev))
- hci_setup_sync(sco, conn->handle);
- else
+ if (lmp_esco_capable(conn->hdev)) {
+ if (use_enhanced_sco_conn(conn->hdev))
+ hci_enhanced_setup_sync(sco, conn->handle);
+ else
+ hci_setup_sync(sco, conn->handle);
+ } else {
hci_add_sco(sco, conn->handle);
+ }
} else {
hci_connect_cfm(sco, status);
hci_conn_del(sco);
@@ -1319,7 +1413,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
}

struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u16 setting)
+ __u16 setting, struct bt_codec *codec)
{
struct hci_conn *acl;
struct hci_conn *sco;
@@ -1344,6 +1438,10 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
hci_conn_hold(sco);

sco->setting = setting;
+ sco->codec.id = codec->id;
+ sco->codec.cid = codec->cid;
+ sco->codec.vid = codec->vid;
+ sco->codec.data_path = codec->data_path;

if (acl->state == BT_CONNECTED &&
(sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1c3018202564..a021f29f12ad 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2236,6 +2236,41 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
hci_dev_unlock(hdev);
}

+static void hci_cs_enhanced_setup_sync_conn(struct hci_dev *hdev, __u8 status)
+{
+ struct hci_cp_enhanced_setup_sync_conn *cp;
+ struct hci_conn *acl, *sco;
+ __u16 handle;
+
+ bt_dev_dbg(hdev, "status 0x%2.2x", status);
+
+ if (!status)
+ return;
+
+ cp = hci_sent_cmd_data(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN);
+ if (!cp)
+ return;
+
+ handle = __le16_to_cpu(cp->handle);
+
+ bt_dev_dbg(hdev, "handle 0x%4.4x", handle);
+
+ hci_dev_lock(hdev);
+
+ acl = hci_conn_hash_lookup_handle(hdev, handle);
+ if (acl) {
+ sco = acl->link;
+ if (sco) {
+ sco->state = BT_CLOSED;
+
+ hci_connect_cfm(sco, status);
+ hci_conn_del(sco);
+ }
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_cs_sniff_mode(struct hci_dev *hdev, __u8 status)
{
struct hci_cp_sniff_mode *cp;
@@ -3715,6 +3750,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cs_setup_sync_conn(hdev, ev->status);
break;

+ case HCI_OP_ENHANCED_SETUP_SYNC_CONN:
+ hci_cs_enhanced_setup_sync_conn(hdev, ev->status);
+ break;
+
case HCI_OP_SNIFF_MODE:
hci_cs_sniff_mode(hdev, ev->status);
break;
@@ -4401,8 +4440,13 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
if (conn->out) {
conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
(hdev->esco_type & EDR_ESCO_MASK);
- if (hci_setup_sync(conn, conn->link->handle))
- goto unlock;
+ if (use_enhanced_sco_conn(hdev)) {
+ if (hci_enhanced_setup_sync(conn, conn->link->handle))
+ goto unlock;
+ } else {
+ if (hci_setup_sync(conn, conn->link->handle))
+ goto unlock;
+ }
}
fallthrough;

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 4b6ee0b302d7..b44d56eb936a 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -240,7 +240,7 @@ static int sco_connect(struct sock *sk)
}

hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
- sco_pi(sk)->setting);
+ sco_pi(sk)->setting, &sco_pi(sk)->codec);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
goto done;
@@ -876,9 +876,9 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
u32 opt;
#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
struct bt_codecs *codecs;
- struct hci_dev *hdev;
__u8 buffer[255];
#endif
+ struct hci_dev *hdev;

BT_DBG("sk %p", sk);

@@ -926,6 +926,15 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
}

sco_pi(sk)->setting = voice.setting;
+ hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src,
+ BDADDR_BREDR);
+ if (!hdev) {
+ err = -EBADFD;
+ break;
+ }
+ if (use_enhanced_sco_conn(hdev) &&
+ voice.setting == BT_VOICE_TRANSPARENT)
+ sco_pi(sk)->codec.id = CODING_FORMAT_TRANSPARENT;
break;

case BT_PKT_STATUS:
--
2.17.1

2021-06-30 09:13:28

Hi Luiz, Marcel,

> -----Original Message-----
> From: Marcel Holtmann <[email protected]>
> Sent: Friday, July 23, 2021 12:21 AM
> To: Luiz Augusto von Dentz <[email protected]>
> Cc: K, Kiran <[email protected]>; [email protected]
> Subject: Re: [PATCH v10 10/10] Bluetooth: Add offload feature under
> experimental flag
>
> Hi Luiz,
>
> >>>>>> Allow user level process to enable / disable codec offload
> >>>>>> feature through mgmt interface. By default offload codec feature
> >>>>>> is disabled.
> >>>>>>
> >>>>>> Signed-off-by: Kiran K <[email protected]>
> >>>>>> Reviewed-by: Chethan T N <[email protected]>
> >>>>>> Reviewed-by: Srivatsa Ravishankar
> >>>>>> <[email protected]>
> >>>>>> ---
> >>>>>> * changes in v10:
> >>>>>> - new patch added to place offload codec feature under
> >>>>>> experimental flag
> >>>>>>
> >>>>>> include/net/bluetooth/hci.h | 4 ++
> >>>>>> net/bluetooth/mgmt.c | 106
> +++++++++++++++++++++++++++++++++++-
> >>>>>> net/bluetooth/sco.c | 10 ++++
> >>>>>> 3 files changed, 119 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/include/net/bluetooth/hci.h
> >>>>>> b/include/net/bluetooth/hci.h index e20318854900..5ca98d9f64dd
> >>>>>> 100644
> >>>>>> --- a/include/net/bluetooth/hci.h
> >>>>>> +++ b/include/net/bluetooth/hci.h
> >>>>>> @@ -331,6 +331,10 @@ enum {
> >>>>>> HCI_CMD_PENDING,
> >>>>>> HCI_FORCE_NO_MITM,
> >>>>>>
> >>>>>> +#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
> >>>>>> + HCI_OFFLOAD_CODECS_ENABLED, #endif
> >>>>>
> >>>>> That is probably a bad idea as it could lead the enum to assume
> >>>>> different values based on what is enabled, besides we don't gain
> >>>>> anything by not having the symbol defined all the time.
> >>>>
> >>>> While this would work with dev_flags which are internal and not API, I
> still don’t like it.
> >>>>
> >>>> There is really no benefit to make this a compile time option. And as far
> as I remember I never said this needs to be compile time. Actually I rather
> have this as an experimental setting so that it can be switched on at runtime.
> Nobody is going to recompile their kernels to test codec offload.
> >>>
> >>> Initially I was with the same opinion, but the problem is the codecs
> >>> are read at init sequence and the experimental flags are set at a
> >>> later stage thus why I suggested a KConfig option until the feature
> >>> is more mature and we can remove the option altogether.
> >>
> >> I am fine with the codec options being read all the time. I mean having an
> experimental option to control the use of offload.
> >
> > Alright, then we don't need the Kconfig after all, the experimental
> > flag will only control the use of the codecs e.g. socketopts would not
> > work if the flag is not enabled I assume?
>
> exactly. It would then return EOPNOTSUPP error. It would be similar to an old
> kernel where this socket option is not available either.

Ack. I will send the changes in the next patchset.

>
> Regards
>
> Marcel

Regards,
Kiran