diff mbox series

[RFC] Bluetooth: don't use ESCO setup for BT_VOICE

Message ID 20220227163430.24694-1-pav@iki.fi
State New
Headers show
Series [RFC] Bluetooth: don't use ESCO setup for BT_VOICE | expand

Commit Message

Pauli Virtanen Feb. 27, 2022, 4:34 p.m. UTC 
According to user reports, how HCI_Enhanced_Setup_Synchronous_Connection
is currently used to establish MSBC connections results to broken audio
on some adapters (QCA6174, mt7921e).

Revert to previous behavior of using HCI_Setup_Synchronous_Connection,
unless the user has explicitly set BT_CODEC sockopt. Since bt_codec
contents come from Core specification, use a separate flag for the
setting.

Fixes: b2af264ad3af ("Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215576
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    Maybe we want to use the ESCO connect setup only when userspace has
    requested the codec offload support.  I don't have any of the broken
    hardware myself, so this is not tested on them.
    
    Alternatively, there should be some driver quirk to indicate the
    enhanced sco connection setup is not broken.

 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_conn.c         | 10 ++++++++--
 net/bluetooth/sco.c              | 22 +++++++---------------
 3 files changed, 16 insertions(+), 17 deletions(-)

Comments

Marcel Holtmann Feb. 27, 2022, 5:36 p.m. UTC | #1 
Hi Pauli,

> According to user reports, how HCI_Enhanced_Setup_Synchronous_Connection
> is currently used to establish MSBC connections results to broken audio
> on some adapters (QCA6174, mt7921e).
> 
> Revert to previous behavior of using HCI_Setup_Synchronous_Connection,
> unless the user has explicitly set BT_CODEC sockopt. Since bt_codec
> contents come from Core specification, use a separate flag for the
> setting.
> 
> Fixes: b2af264ad3af ("Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215576
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
> 
> Notes:
>    Maybe we want to use the ESCO connect setup only when userspace has
>    requested the codec offload support.  I don't have any of the broken
>    hardware myself, so this is not tested on them.
> 
>    Alternatively, there should be some driver quirk to indicate the
>    enhanced sco connection setup is not broken.

yes, these needs to be marked as my hardware is broken.
diff mbox series

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index dd8840e70e25..3a6a3b80368c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -717,6 +717,7 @@  struct hci_conn {
 
 	struct hci_conn	*link;
 	struct bt_codec codec;
+	bool		esco_setup;
 
 	void (*connect_cfm_cb)	(struct hci_conn *conn, u8 status);
 	void (*security_cfm_cb)	(struct hci_conn *conn, u8 status);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index bd669c95b9a7..0aa7f822de32 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -481,7 +481,7 @@  static bool hci_setup_sync_conn(struct hci_conn *conn, __u16 handle)
 
 bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
-	if (enhanced_sco_capable(conn->hdev))
+	if (enhanced_sco_capable(conn->hdev) && conn->esco_setup)
 		return hci_enhanced_setup_sync_conn(conn, handle);
 
 	return hci_setup_sync_conn(conn, handle);
@@ -1477,7 +1477,13 @@  struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
 	hci_conn_hold(sco);
 
 	sco->setting = setting;
-	sco->codec = *codec;
+	if (codec) {
+		sco->codec = *codec;
+		sco->esco_setup = true;
+	} else {
+		memset(&sco->codec, 0, sizeof(sco->codec));
+		sco->esco_setup = false;
+	}
 
 	if (acl->state == BT_CONNECTED &&
 	    (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 8eabf41b2993..e78063d65c1a 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -70,6 +70,7 @@  struct sco_pinfo {
 	__u16		setting;
 	__u8		cmsg_mask;
 	struct bt_codec codec;
+	bool		esco_setup;
 	struct sco_conn	*conn;
 };
 
@@ -239,6 +240,7 @@  static int sco_connect(struct hci_dev *hdev, struct sock *sk)
 {
 	struct sco_conn *conn;
 	struct hci_conn *hcon;
+	struct bt_codec *codec;
 	int err, type;
 
 	BT_DBG("%pMR -> %pMR", &sco_pi(sk)->src, &sco_pi(sk)->dst);
@@ -252,8 +254,9 @@  static int sco_connect(struct hci_dev *hdev, struct sock *sk)
 	    (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev)))
 		return -EOPNOTSUPP;
 
+	codec = sco_pi(sk)->esco_setup ? &sco_pi(sk)->codec : NULL;
 	hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
-			       sco_pi(sk)->setting, &sco_pi(sk)->codec);
+			       sco_pi(sk)->setting, codec);
 	if (IS_ERR(hcon))
 		return PTR_ERR(hcon);
 
@@ -496,10 +499,7 @@  static struct sock *sco_sock_alloc(struct net *net, struct socket *sock,
 	sk->sk_state    = BT_OPEN;
 
 	sco_pi(sk)->setting = BT_VOICE_CVSD_16BIT;
-	sco_pi(sk)->codec.id = BT_CODEC_CVSD;
-	sco_pi(sk)->codec.cid = 0xffff;
-	sco_pi(sk)->codec.vid = 0xffff;
-	sco_pi(sk)->codec.data_path = 0x00;
+	sco_pi(sk)->esco_setup = false;
 
 	bt_sock_link(&sco_sk_list, sk);
 	return sk;
@@ -879,16 +879,7 @@  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 (enhanced_sco_capable(hdev) &&
-		    voice.setting == BT_VOICE_TRANSPARENT)
-			sco_pi(sk)->codec.id = BT_CODEC_TRANSPARENT;
-		hci_dev_put(hdev);
+		sco_pi(sk)->esco_setup = false;
 		break;
 
 	case BT_PKT_STATUS:
@@ -951,6 +942,7 @@  static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
 		}
 
 		sco_pi(sk)->codec = codecs->codecs[0];
+		sco_pi(sk)->esco_setup = true;
 		hci_dev_put(hdev);
 		break;