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.
>
> Thanks,
>
> Pavel.
>> - max_size_bw = num_channels(chmask) * ssize *
>> - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
>> + max_size_bw = num_channels(chmask) * ssize * spf;
>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>> max_size_ep));
>>
