Dne 05. 10. 21 v 12:13 Jerome Brunet napsal(a):
On Tue 05 Oct 2021 at 12:00, Pavel Hofman <pavel.hofman@xxxxxxxxxxx> wrote:
Dne 05. 10. 21 v 11:37 Jerome Brunet napsal(a):
This fixes the wMaxPacketSize of the audio gadget so it is in line with
USB Audio Format specification - section 2.3.1.2.1
Cc: Jack Pham <jackp@xxxxxxxxxxxxxx>
Cc: Pavel Hofman <pavel.hofman@xxxxxxxxxxx>
Fixes: e89bb4288378 ("usb: gadget: u_audio: add real feedback implementation")
Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
---
There was a mistake in my previous mail, rounding depends on the
synchronisation, not the stream direction.
drivers/usb/gadget/function/f_uac2.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index ae29ff2b2b68..c152efa30def 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -554,7 +554,7 @@ static int set_ep_max_packet_size(const struct f_uac2_opts *uac2_opts,
struct usb_endpoint_descriptor *ep_desc,
enum usb_device_speed speed, bool is_playback)
{
- int chmask, srate, ssize;
+ int chmask, srate, ssize, spf;
u16 max_size_bw, max_size_ep;
unsigned int factor;
@@ -584,11 +584,12 @@ static int set_ep_max_packet_size(const struct
f_uac2_opts *uac2_opts,
ssize = uac2_opts->c_ssize;
}
- if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
- srate = srate * (1000 + uac2_opts->fb_max) / 1000;
+ if (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ADAPTIVE)
+ spf = DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
+ else
+ spf = (srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1;
Please correct me if I am wrong, but does the change mean that
uac2_opts->c_sync value has also impact on playback (EP-IN)
wMaxPacketSize?
Duh :( - Thanks for catching this ! we only support async for playback
I guess you get the idea, I meant the rounding depends on the sync mode:
ADAPTIVE: spf = DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
ASYNC: spf = (srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1;
The important thing that you should round down for async (not up, as in
the patch you have sent)
Here is quick example with USB full speed
- ADAPTIVE:
* 48kHz: 48 samples/SIP (Service Interval Packet)
* 44.1kHz: max 45 samples/SIP
- ASYNC
* 48kHz: small SIP=47samples - big SIP=49samples
* 44.1kHz small SIP=44samples - big SIP=45samples
Your initial patch would not be correct for ASYNC@44.1kHz.
I think it would give a maximum size (big SIP) of 46 samples instead of
45.
I am sorry I do not understand. You mention chapter 2.3.1.2.1 (I assume
it is SERVICE INTERVAL PACKET SIZE CALCULATION in Fmts30-Errata.pdf).
IIUC that chapter does not deal with async mode because exact samplerate
values converted to sample numbers are used there. How does your new
calculation take into account the upper range of the async rate, now
increased to +25% by your second patch? The max packet size calculation
is done prior to "tweaking" the rate via async feedback, IMO it should
logically take into account the maximum possible increase (which the
previous algorithm did via the fb_max (always > 0) adjustment).
Maybe there is a difference in UAC3 which the Fmts30 specs seem to
describe, I do not know. I just do not see how the possible increase of
packet size in async mode fits into this calculation.
Thanks a lot,
Pavel.
