Hi Pauli,
On Wed, Jun 21, 2023 at 1:33 PM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Hi Luiz,
>
> ke, 2023-06-21 kello 12:54 -0700, Luiz Augusto von Dentz kirjoitti:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> >
> > The ISO Interval on CIS Established Event uses 1.25 ms slots:
> >
> > BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
> > page 2304:
> >
> > Time = N * 1.25 ms
> >
> > In addition to that this always update the QoS settings based on CIS
> > Established Event.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> > ---
> > net/bluetooth/hci_event.c | 36 +++++++++++++++++++++++-------------
> > 1 file changed, 23 insertions(+), 13 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index b1aefe4bb751..6fca6d9f1b34 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -6822,6 +6822,7 @@ static void hci_le_cis_estabilished_evt(struct hci_dev *hdev, void *data,
> > {
> > struct hci_evt_le_cis_established *ev = data;
> > struct hci_conn *conn;
> > + struct bt_iso_qos *qos;
> > bool pending = false;
> > u16 handle = __le16_to_cpu(ev->handle);
> >
> > @@ -6846,21 +6847,30 @@ static void hci_le_cis_estabilished_evt(struct hci_dev *hdev, void *data,
> >
> > pending = test_and_clear_bit(HCI_CONN_CREATE_CIS, &conn->flags);
> >
> > - if (conn->role == HCI_ROLE_SLAVE) {
> > - __le32 interval;
> > + qos = &conn->iso_qos;
> >
> > - memset(&interval, 0, sizeof(interval));
> > + /* Convert ISO Interval (1.25 ms slots) to latency (ms) */
> > + qos->ucast.in.latency = le16_to_cpu(ev->interval) * 125 / 100;
> > + /* Convert ISO Interval (1.25 ms slots) to latency (ms) */
> > + qos->ucast.out.latency = le16_to_cpu(ev->interval) * 125 / 100;
> >
> > - memcpy(&interval, ev->c_latency, sizeof(ev->c_latency));
> > - conn->iso_qos.ucast.in.interval = le32_to_cpu(interval);
> > - memcpy(&interval, ev->p_latency, sizeof(ev->p_latency));
> > - conn->iso_qos.ucast.out.interval = le32_to_cpu(interval);
> > - conn->iso_qos.ucast.in.latency = le16_to_cpu(ev->interval);
> > - conn->iso_qos.ucast.out.latency = le16_to_cpu(ev->interval);
> > - conn->iso_qos.ucast.in.sdu = le16_to_cpu(ev->c_mtu);
> > - conn->iso_qos.ucast.out.sdu = le16_to_cpu(ev->p_mtu);
> > - conn->iso_qos.ucast.in.phy = ev->c_phy;
> > - conn->iso_qos.ucast.out.phy = ev->p_phy;
> > + switch (conn->role) {
> > + case HCI_ROLE_SLAVE:
> > + qos->ucast.in.interval = get_unaligned_le24(ev->c_latency);
> > + qos->ucast.out.interval = get_unaligned_le24(ev->p_latency);
> > + qos->ucast.in.sdu = le16_to_cpu(ev->c_mtu);
> > + qos->ucast.out.sdu = le16_to_cpu(ev->p_mtu);
> > + qos->ucast.in.phy = ev->c_phy;
> > + qos->ucast.out.phy = ev->p_phy;
> > + break;
>
> Are the ucast.latency and ucast.interval the right way around here?
>
> When I trying to use this in userspace, I expected ucast.interval
> contains the ISO interval, because in Set CIG Parameters we use
> ucast.interval to specify the SDU_Interval, and ucast.latency is used
> for the Transport_Latency_C_To_P and Transport_Latency_P_To_C.
>
> With real numbers the event (AX210<->AX210) looks like this
>
> > HCI Event: LE Meta Event (0x3e) plen 29 #3493 [hci0] 486.978955
> LE Connected Isochronous Stream Established (0x19)
> Status: Success (0x00)
> Connection Handle: 2560
> CIG Synchronization Delay: 4020 us (0x000fb4)
> CIS Synchronization Delay: 4020 us (0x000fb4)
> Central to Peripheral Latency: 94020 us (0x016f44)
> Peripheral to Central Latency: 94020 us (0x016f44)
No idea why these values look like this, they are not what I expect
which is to match what the Central configured with Set CIG Parameters.
> Central to Peripheral PHY: LE 2M (0x02)
> Peripheral to Central PHY: LE 2M (0x02)
> Number of Subevents: 2
> Central to Peripheral Burst Number: 1
> Peripheral to Central Burst Number: 1
> Central to Peripheral Flush Timeout: 10
> Peripheral to Central Flush Timeout: 10
> Central to Peripheral MTU: 240
> Peripheral to Central MTU: 120
> ISO Interval: 8
These seems to be fine, so I wonder what is going on with CIS
Established event, this is what we are doing in the emulator:
/* TODO: Figure out if these values makes sense */
memcpy(evt.cig_sync_delay, le_cig->params.c_interval,
sizeof(le_cig->params.c_interval));
memcpy(evt.cis_sync_delay, le_cig->params.p_interval,
sizeof(le_cig->params.p_interval));
memcpy(evt.c_latency, &le_cig->params.c_interval,
sizeof(le_cig->params.c_interval));
memcpy(evt.p_latency, &le_cig->params.p_interval,
sizeof(le_cig->params.p_interval));
evt.c_phy = le_cig->cis[cis_idx].c_phy;
evt.p_phy = le_cig->cis[cis_idx].p_phy;
evt.nse = 0x01;
evt.c_bn = 0x01;
evt.p_bn = 0x01;
evt.c_ft = 0x01;
evt.p_ft = 0x01;
evt.c_mtu = le_cig->cis[cis_idx].c_sdu;
evt.p_mtu = le_cig->cis[cis_idx].p_sdu;
evt.interval = (le_cig->params.c_latency + 1) / 1.25;
>
> > + case HCI_ROLE_MASTER:
> > + qos->ucast.out.interval = get_unaligned_le24(ev->c_latency);
> > + qos->ucast.in.interval = get_unaligned_le24(ev->p_latency);
> > + qos->ucast.out.sdu = le16_to_cpu(ev->c_mtu);
> > + qos->ucast.in.sdu = le16_to_cpu(ev->p_mtu);
> > + qos->ucast.out.phy = ev->c_phy;
> > + qos->ucast.in.phy = ev->p_phy;
> > + break;
> > }
> >
> > if (!ev->status) {
>
> --
> Pauli Virtanen
--
Luiz Augusto von Dentz
