* usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
@ 2021-07-13 10:22 Pavel Hofman
2021-07-13 10:49 ` Greg KH
2021-07-13 12:05 ` Jerome Brunet
0 siblings, 2 replies; 11+ messages in thread
From: Pavel Hofman @ 2021-07-13 10:22 UTC (permalink / raw)
To: linux-usb; +Cc: Jerome Brunet, Ruslan Bilovol
Hi,
I am testing the three Ruslan's async feedback patches as modified by
Jerome and I hit a regression in windows 10 enumeration.
Ruslan posted an already accepted patch
https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
which allowed win10 enumeration.
Ruslan's async feedback patchset kept the change:
https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 72b42f8..91b22fb 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
f_uac2_opts *uac2_opts,
max_size_bw = num_channels(chmask) * ssize *
((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
+
+ if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
+ max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
+
ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
max_size_ep));
Jerome's rebase patch which was accepted recently changed the
functionality to the original code:
https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 321e6c05ba93..ae29ff2b2b68 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -584,8 +584,11 @@ 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;
+
max_size_bw = num_channels(chmask) * ssize *
- ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
+ DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
max_size_ep));
With this version my Win10 does not enumerate the USB AUDIO device, even
if it has only EP-IN capability (i.e. is_playback = true). For my setup
the EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version,
causing win10 reporting "The specified range could not be found in the
range list."
A simple change makes Win10 enumerate both playback-only as well as
duplex playback/capture async device:
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index ae29ff2b2b68..5ba778814796 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -588,7 +588,7 @@ static int set_ep_max_packet_size(const struct
f_uac2_opts *uac2_opts,
srate = srate * (1000 + uac2_opts->fb_max) / 1000;
max_size_bw = num_channels(chmask) * ssize *
- DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval -
1)));
+ (DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval
- 1))) + 1);
ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
max_size_ep));
I do not know if this is the most correct solution but IMO a similar
patch should be applied. I can send a proper patch mail if this solution
is acceptable.
Thanks a lot,
Pavel.
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-13 10:22 usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation Pavel Hofman
@ 2021-07-13 10:49 ` Greg KH
2021-07-13 12:05 ` Jerome Brunet
1 sibling, 0 replies; 11+ messages in thread
From: Greg KH @ 2021-07-13 10:49 UTC (permalink / raw)
To: Pavel Hofman; +Cc: linux-usb, Jerome Brunet, Ruslan Bilovol
On Tue, Jul 13, 2021 at 12:22:38PM +0200, Pavel Hofman wrote:
> Hi,
>
> I am testing the three Ruslan's async feedback patches as modified by Jerome
> and I hit a regression in windows 10 enumeration.
>
> Ruslan posted an already accepted patch https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
We do not use github for kernel stuff, just reference the git commit id
properly and all is good.
> which allowed win10 enumeration.
>
> Ruslan's async feedback patchset kept the change: https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
I do not understand this reference at all.
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index 72b42f8..91b22fb 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
> f_uac2_opts *uac2_opts,
>
> max_size_bw = num_channels(chmask) * ssize *
> ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> +
> + if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> + max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
> +
> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
> max_size_ep));
>
>
> Jerome's rebase patch which was accepted recently changed the functionality
> to the original code:
> https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index 321e6c05ba93..ae29ff2b2b68 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -584,8 +584,11 @@ 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;
> +
> max_size_bw = num_channels(chmask) * ssize *
> - ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> + DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
> max_size_ep));
>
> With this version my Win10 does not enumerate the USB AUDIO device, even if
> it has only EP-IN capability (i.e. is_playback = true). For my setup the
> EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version, causing
> win10 reporting "The specified range could not be found in the range list."
>
> A simple change makes Win10 enumerate both playback-only as well as duplex
> playback/capture async device:
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index ae29ff2b2b68..5ba778814796 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -588,7 +588,7 @@ static int set_ep_max_packet_size(const struct
> f_uac2_opts *uac2_opts,
> srate = srate * (1000 + uac2_opts->fb_max) / 1000;
>
> max_size_bw = num_channels(chmask) * ssize *
> - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval -
> 1)));
> + (DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval -
> 1))) + 1);
> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
> max_size_ep));
>
>
> I do not know if this is the most correct solution but IMO a similar patch
> should be applied. I can send a proper patch mail if this solution is
> acceptable.
Just send the patch and we will go from there.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-13 10:22 usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation Pavel Hofman
2021-07-13 10:49 ` Greg KH
@ 2021-07-13 12:05 ` Jerome Brunet
2021-07-13 12:32 ` Felipe Balbi
2021-07-13 13:16 ` Pavel Hofman
1 sibling, 2 replies; 11+ messages in thread
From: Jerome Brunet @ 2021-07-13 12:05 UTC (permalink / raw)
To: Pavel Hofman, linux-usb; +Cc: Ruslan Bilovol
On Tue 13 Jul 2021 at 12:22, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
> Hi,
>
> I am testing the three Ruslan's async feedback patches as modified by
> Jerome and I hit a regression in windows 10 enumeration.
>
> Ruslan posted an already accepted patch
> https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
> which allowed win10 enumeration.
>
> Ruslan's async feedback patchset kept the change:
> https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index 72b42f8..91b22fb 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
> f_uac2_opts *uac2_opts,
>
> max_size_bw = num_channels(chmask) * ssize *
> ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> +
> + if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> + max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
> +
> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
> max_size_ep));
>
>
> Jerome's rebase patch which was accepted recently changed the functionality
> to the original code:
> https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index 321e6c05ba93..ae29ff2b2b68 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -584,8 +584,11 @@ 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;
> +
> max_size_bw = num_channels(chmask) * ssize *
> - ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> + DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
> max_size_ep));
>
> With this version my Win10 does not enumerate the USB AUDIO device, even if
> it has only EP-IN capability (i.e. is_playback = true). For my setup the
> EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version, causing
> win10 reporting "The specified range could not be found in the range list."
>
Maybe I am lacking USB expertize, but is there any reason why a 192bytes
maximum packet size should be considered invalid ? Just from your
comment, I can't figure it out.
It would help if you could detail the parameters you started your gadget
with (rate, format, channel number)
IIRC, Ruslan initial patchset reserved a fixed additional bandwidth
(around 20% I think) to deal with the explicit feedback.
The idea is that, 99.9% of the time, all you need is the ability to fit
one more sample in each packet. This is should be what the default
setting provides (up to 192kHz). If it does not do that, I made mistake.
The setting configurable because I was trying to avoid wasting USB
bandwith but still support poor quality platforms where 1 extra sample
is not enough (I think Ruslan mentioned having seen such system)
> A simple change makes Win10 enumerate both playback-only as well as duplex
> playback/capture async device:
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index ae29ff2b2b68..5ba778814796 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -588,7 +588,7 @@ static int set_ep_max_packet_size(const struct
> f_uac2_opts *uac2_opts,
> srate = srate * (1000 + uac2_opts->fb_max) / 1000;
>
> max_size_bw = num_channels(chmask) * ssize *
> - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval -
> 1)));
> + (DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval -
> 1))) + 1);
I don't really understand why you should add 1 here and how it correlate to
your remark above ?
> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
> max_size_ep));
>
>
> I do not know if this is the most correct solution but IMO a similar patch
> should be applied. I can send a proper patch mail if this solution is
> acceptable.
>
> Thanks a lot,
>
> Pavel.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-13 12:05 ` Jerome Brunet
@ 2021-07-13 12:32 ` Felipe Balbi
2021-07-13 13:16 ` Pavel Hofman
1 sibling, 0 replies; 11+ messages in thread
From: Felipe Balbi @ 2021-07-13 12:32 UTC (permalink / raw)
To: Jerome Brunet, Pavel Hofman, linux-usb; +Cc: Ruslan Bilovol
[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]
Hi,
Jerome Brunet <jbrunet@baylibre.com> writes:
>> I am testing the three Ruslan's async feedback patches as modified by
>> Jerome and I hit a regression in windows 10 enumeration.
>>
>> Ruslan posted an already accepted patch
>> https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
>> which allowed win10 enumeration.
>>
>> Ruslan's async feedback patchset kept the change:
>> https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
>>
>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>> b/drivers/usb/gadget/function/f_uac2.c
>> index 72b42f8..91b22fb 100644
>> --- a/drivers/usb/gadget/function/f_uac2.c
>> +++ b/drivers/usb/gadget/function/f_uac2.c
>> @@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
>> f_uac2_opts *uac2_opts,
>>
>> max_size_bw = num_channels(chmask) * ssize *
>> ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>> +
>> + if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
>> + max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
>> +
>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>> max_size_ep));
>>
>>
>> Jerome's rebase patch which was accepted recently changed the functionality
>> to the original code:
>> https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
>>
>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>> b/drivers/usb/gadget/function/f_uac2.c
>> index 321e6c05ba93..ae29ff2b2b68 100644
>> --- a/drivers/usb/gadget/function/f_uac2.c
>> +++ b/drivers/usb/gadget/function/f_uac2.c
>> @@ -584,8 +584,11 @@ 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;
>> +
>> max_size_bw = num_channels(chmask) * ssize *
>> - ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>> + DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>> max_size_ep));
>>
>> With this version my Win10 does not enumerate the USB AUDIO device, even if
>> it has only EP-IN capability (i.e. is_playback = true). For my setup the
>> EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version, causing
>> win10 reporting "The specified range could not be found in the range list."
>>
>
> Maybe I am lacking USB expertize, but is there any reason why a 192bytes
> maximum packet size should be considered invalid ? Just from your
> comment, I can't figure it out.
it sounds to me like one part of the descriptor claims 192 while another
claims 196, then there is a mismatch and Windows is ignoring the
interface. A quick dump of the descriptors would prove this.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-13 12:05 ` Jerome Brunet
2021-07-13 12:32 ` Felipe Balbi
@ 2021-07-13 13:16 ` Pavel Hofman
2021-07-13 13:28 ` Pavel Hofman
2021-07-15 9:39 ` Jerome Brunet
1 sibling, 2 replies; 11+ messages in thread
From: Pavel Hofman @ 2021-07-13 13:16 UTC (permalink / raw)
To: Jerome Brunet, linux-usb; +Cc: Ruslan Bilovol, Felipe Balbi
[-- Attachment #1: Type: text/plain, Size: 5372 bytes --]
Dne 13. 07. 21 v 14:05 Jerome Brunet napsal(a):
>
> On Tue 13 Jul 2021 at 12:22, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>
>> Hi,
>>
>> I am testing the three Ruslan's async feedback patches as modified by
>> Jerome and I hit a regression in windows 10 enumeration.
>>
>> Ruslan posted an already accepted patch
>> https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
>> which allowed win10 enumeration.
>>
>> Ruslan's async feedback patchset kept the change:
>> https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
>>
>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>> b/drivers/usb/gadget/function/f_uac2.c
>> index 72b42f8..91b22fb 100644
>> --- a/drivers/usb/gadget/function/f_uac2.c
>> +++ b/drivers/usb/gadget/function/f_uac2.c
>> @@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
>> f_uac2_opts *uac2_opts,
>>
>> max_size_bw = num_channels(chmask) * ssize *
>> ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>> +
>> + if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
>> + max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
>> +
>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>> max_size_ep));
>>
>>
>> Jerome's rebase patch which was accepted recently changed the functionality
>> to the original code:
>> https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
>>
>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>> b/drivers/usb/gadget/function/f_uac2.c
>> index 321e6c05ba93..ae29ff2b2b68 100644
>> --- a/drivers/usb/gadget/function/f_uac2.c
>> +++ b/drivers/usb/gadget/function/f_uac2.c
>> @@ -584,8 +584,11 @@ 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;
>> +
>> max_size_bw = num_channels(chmask) * ssize *
>> - ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>> + DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>> max_size_ep));
>>
>> With this version my Win10 does not enumerate the USB AUDIO device, even if
>> it has only EP-IN capability (i.e. is_playback = true). For my setup the
>> EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version, causing
>> win10 reporting "The specified range could not be found in the range list."
>>
>
> Maybe I am lacking USB expertize, but is there any reason why a 192bytes
> maximum packet size should be considered invalid ? Just from your
> comment, I can't figure it out.
>
> It would help if you could detail the parameters you started your gadget
> with (rate, format, channel number)
>
> IIRC, Ruslan initial patchset reserved a fixed additional bandwidth
> (around 20% I think) to deal with the explicit feedback.
>
> The idea is that, 99.9% of the time, all you need is the ability to fit
> one more sample in each packet. This is should be what the default
> setting provides (up to 192kHz). If it does not do that, I made mistake.
>
> The setting configurable because I was trying to avoid wasting USB
> bandwith but still support poor quality platforms where 1 extra sample
> is not enough (I think Ruslan mentioned having seen such system)
I am absolutely no USB expert. What I did was testing Jerome's patchset
and win10 refused to enumerate, even with the most trivial configuration
c_srate=p_srate=48000, 16bits, capture 2ch, playback 0ch which
configures no EP-OUT and no feedback EP-IN. To find the cause I went
back one patch (HEAD^) and win10 happily enumerated this no-EP-OUT
configuration. So I compared the usb config dump - see attached files
from Theosycon USB Descriptor Dumper - see the two attached files, named
after commit IDs.
The dumps differ in only one parameter EP-IN 1 wMaxPacketSize, for both
HS and the dumped "Other Speed Configuration Descriptor" i.e. FS:
diff 650f7f40dc1691a8ab4d1dc411f8f327b36e8d14.txt
cb1c270600b2c6f55f55f227775aaddf2cc78bed.txt
185c185
< 0x00C4 wMaxPacketSize (1 x 196 bytes)
---
> 0x00C0 wMaxPacketSize (1 x 192 bytes)
339c339
< 0x00C4 wMaxPacketSize (1 x 196 bytes)
---
> 0x00C0 wMaxPacketSize (1 x 192 bytes)
So I looked at the patch and saw the changed wMaxPacketSize calculation.
Adding +1 yielded what the original Ruslan's code yielded - one more
sample (i.e. 196 bytes instead of 192bytes). Therefore I put it in the
patch here. This value is accepted by win10.
I do not know how windows uac2 driver verifies validity of
wMaxPacketSize but clearly 196bytes (+1 sample) is accepted while
192bytes is refused. Linux does not care, just like Ruslan described in
his commit 789ea77310f020.
I checked duplex with EP-OUT enabled (i.e. with the feedback EP-IN), the
"new" calculation works OK in win10 (while the Jerome's values were
refused).
I really do not know the correct code for calculating the wMaxPacketSize
to satisfy the windows driver, but what I am posting works. I'll be
happy if someone knowledgeable fixes my layman quickfix.
Best regards,
Pavel.
[-- Attachment #2: 650f7f40dc1691a8ab4d1dc411f8f327b36e8d14.txt --]
[-- Type: text/plain, Size: 9563 bytes --]
Information for device Linux USB Audio Gadget (VID=0x1D6B PID=0x0101):
------------------------------
Connection Information:
------------------------------
Device current bus speed: HighSpeed
Device supports USB 1.1 specification
Device supports USB 2.0 specification
Device address: 0x0001
Current configuration value: 0x01
Number of open pipes: 0
------------------------------
Device Descriptor:
------------------------------
0x12 bLength
0x01 bDescriptorType
0x0200 bcdUSB
0xEF bDeviceClass (Miscellaneous device)
0x02 bDeviceSubClass
0x01 bDeviceProtocol
0x40 bMaxPacketSize0 (64 bytes)
0x1D6B idVendor
0x0101 idProduct
0x0513 bcdDevice
0x01 iManufacturer "Linux 5.13.1-v8+ with fe980000.usb"
0x02 iProduct "Linux USB Audio Gadget"
0x00 iSerialNumber
0x01 bNumConfigurations
Device Qualifier Descriptor:
------------------------------
0x0A bLength
0x06 bDescriptorType
0x0200 bcdUSB
0xEF bDeviceClass (Miscellaneous device)
0x02 bDeviceSubClass
0x01 bDeviceProtocol
0x40 bMaxPacketSize0 (64 bytes)
0x01 bNumConfigurations
0x00 bReserved
-------------------------
Configuration Descriptor:
-------------------------
0x09 bLength
0x02 bDescriptorType
0x007F wTotalLength (127 bytes)
0x02 bNumInterfaces
0x01 bConfigurationValue
0x00 iConfiguration
0xC0 bmAttributes (Self-powered Device)
0x01 bMaxPower (2 mA)
Interface Association Descriptor:
------------------------------
0x08 bLength
0x0B bDescriptorType
0x00 bFirstInterface
0x02 bInterfaceCount
0x01 bFunctionClass (Audio Device Class)
0x00 bFunctionSubClass
0x20 bFunctionProtocol (Audio Protocol IP version 2.00)
0x04 iFunction "Source/Sink"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x00 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x01 bInterfaceSubClass (Audio Control Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x05 iInterface "Topology Control"
AC Interface Header Descriptor:
------------------------------
0x09 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x0200 bcdADC
0x08 bCategory (IO_BOX)
0x002E wTotalLength (46 bytes)
0x00 bmControls
AC Clock Source Descriptor:
------------------------------
0x08 bLength
0x24 bDescriptorType
0x0A bDescriptorSubtype
0x03 bClockID
0x01 bmAttributes
0x01 bmControls
Clock Frequency Control - read only
0x00 bAssocTerminal
0x06 iClockSource "48000Hz"
AC Input Terminal Descriptor:
------------------------------
0x11 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bTerminalID
0x0200 wTerminalType (Input Undefined)
0x00 bAssocTerminal
0x03 bCSourceID
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
0x03 bmControls
Copy Protect Control - host programmable
0x09 iTerminal "USBD Out"
AC Output Terminal Descriptor:
------------------------------
0x0C bLength
0x24 bDescriptorType
0x03 bDescriptorSubtype
0x02 bTerminalID
0x0101 wTerminalType (USB Streaming)
0x00 bAssocTerminal
0x01 bSourceID
0x03 bCSourceID
0x0003 bmControls
Copy Protect Control - host programmable
0x0A iTerminal "USBH In"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0E iInterface "Capture Inactive"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x01 bAlternateSetting
0x01 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0F iInterface "Capture Active"
AS Interface Descriptor:
------------------------------
0x10 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x02 bTerminalLink
0x00 bmControls
0x01 bFormatType (FORMAT_TYPE_1)
0x00000001 bmFormats
PCM
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
AS Format Type 1 Descriptor:
------------------------------
0x06 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bFormatType (FORMAT_TYPE_1)
0x02 bSubslotSize
0x10 bBitResolution (16 bits per sample)
Endpoint Descriptor:
------------------------------
0x07 bLength
0x05 bDescriptorType
0x81 bEndpointAddress (IN endpoint 1)
0x05 bmAttributes (Transfer: Isochronous / Synch: Asynchronous / Usage: Data)
0x00C4 wMaxPacketSize (1 x 196 bytes)
0x04 bInterval (8 microframes)
AS Isochronous Data Endpoint Descriptor:
------------------------------
0x08 bLength
0x25 bDescriptorType
0x01 bDescriptorSubtype
0x00 bmAttributes
0x00 bmControls
0x00 bLockDelayUnits (undefined)
0x0000 wLockDelay
-------------------------------------
Other Speed Configuration Descriptor:
-------------------------------------
0x09 bLength
0x07 bDescriptorType
0x007F wTotalLength (127 bytes)
0x02 bNumInterfaces
0x01 bConfigurationValue
0x00 iConfiguration
0xC0 bmAttributes (Self-powered Device)
0x01 bMaxPower (2 mA)
Interface Association Descriptor:
------------------------------
0x08 bLength
0x0B bDescriptorType
0x00 bFirstInterface
0x02 bInterfaceCount
0x01 bFunctionClass (Audio Device Class)
0x00 bFunctionSubClass
0x20 bFunctionProtocol (Audio Protocol IP version 2.00)
0x04 iFunction "Source/Sink"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x00 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x01 bInterfaceSubClass (Audio Control Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x05 iInterface "Topology Control"
AC Interface Header Descriptor:
------------------------------
0x09 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x0200 bcdADC
0x08 bCategory (IO_BOX)
0x002E wTotalLength (46 bytes)
0x00 bmControls
AC Clock Source Descriptor:
------------------------------
0x08 bLength
0x24 bDescriptorType
0x0A bDescriptorSubtype
0x03 bClockID
0x01 bmAttributes
0x01 bmControls
Clock Frequency Control - read only
0x00 bAssocTerminal
0x06 iClockSource "48000Hz"
AC Input Terminal Descriptor:
------------------------------
0x11 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bTerminalID
0x0200 wTerminalType (Input Undefined)
0x00 bAssocTerminal
0x03 bCSourceID
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
0x03 bmControls
Copy Protect Control - host programmable
0x09 iTerminal "USBD Out"
AC Output Terminal Descriptor:
------------------------------
0x0C bLength
0x24 bDescriptorType
0x03 bDescriptorSubtype
0x02 bTerminalID
0x0101 wTerminalType (USB Streaming)
0x00 bAssocTerminal
0x01 bSourceID
0x03 bCSourceID
0x0003 bmControls
Copy Protect Control - host programmable
0x0A iTerminal "USBH In"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0E iInterface "Capture Inactive"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x01 bAlternateSetting
0x01 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0F iInterface "Capture Active"
AS Interface Descriptor:
------------------------------
0x10 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x02 bTerminalLink
0x00 bmControls
0x01 bFormatType (FORMAT_TYPE_1)
0x00000001 bmFormats
PCM
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
AS Format Type 1 Descriptor:
------------------------------
0x06 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bFormatType (FORMAT_TYPE_1)
0x02 bSubslotSize
0x10 bBitResolution (16 bits per sample)
Endpoint Descriptor:
------------------------------
0x07 bLength
0x05 bDescriptorType
0x81 bEndpointAddress (IN endpoint 1)
0x05 bmAttributes (Transfer: Isochronous / Synch: Asynchronous / Usage: Data)
0x00C4 wMaxPacketSize (1 x 196 bytes)
0x01 bInterval (1 frames)
AS Isochronous Data Endpoint Descriptor:
------------------------------
0x08 bLength
0x25 bDescriptorType
0x01 bDescriptorSubtype
0x00 bmAttributes
0x00 bmControls
0x00 bLockDelayUnits (undefined)
0x0000 wLockDelay
Microsoft OS Descriptor is not available. Error code: 0x0000001F
--------------------------------
String Descriptor Table
--------------------------------
Index LANGID String
0x00 0x0000 0x0409
0x01 0x0409 "Linux 5.13.1-v8+ with fe980000.usb"
0x02 0x0409 "Linux USB Audio Gadget"
0x04 0x0409 "Source/Sink"
0x05 0x0409 "Topology Control"
0x06 0x0409 "48000Hz"
0x09 0x0409 "USBD Out"
0x0A 0x0409 "USBH In"
0x0E 0x0409 "Capture Inactive"
0x0F 0x0409 "Capture Active"
------------------------------
Connection path for device:
Standard Enhanced PCI to USB Host Controller
Root Hub
Linux USB Audio Gadget (VID=0x1D6B PID=0x0101) Port: 1
Running on: Windows 10 or greater (Build Version 19041)
Brought to you by TDD v2.17.0, Feb 23 2021, 14:04:02
[-- Attachment #3: cb1c270600b2c6f55f55f227775aaddf2cc78bed.txt --]
[-- Type: text/plain, Size: 9563 bytes --]
Information for device Linux USB Audio Gadget (VID=0x1D6B PID=0x0101):
------------------------------
Connection Information:
------------------------------
Device current bus speed: HighSpeed
Device supports USB 1.1 specification
Device supports USB 2.0 specification
Device address: 0x0001
Current configuration value: 0x01
Number of open pipes: 0
------------------------------
Device Descriptor:
------------------------------
0x12 bLength
0x01 bDescriptorType
0x0200 bcdUSB
0xEF bDeviceClass (Miscellaneous device)
0x02 bDeviceSubClass
0x01 bDeviceProtocol
0x40 bMaxPacketSize0 (64 bytes)
0x1D6B idVendor
0x0101 idProduct
0x0513 bcdDevice
0x01 iManufacturer "Linux 5.13.1-v8+ with fe980000.usb"
0x02 iProduct "Linux USB Audio Gadget"
0x00 iSerialNumber
0x01 bNumConfigurations
Device Qualifier Descriptor:
------------------------------
0x0A bLength
0x06 bDescriptorType
0x0200 bcdUSB
0xEF bDeviceClass (Miscellaneous device)
0x02 bDeviceSubClass
0x01 bDeviceProtocol
0x40 bMaxPacketSize0 (64 bytes)
0x01 bNumConfigurations
0x00 bReserved
-------------------------
Configuration Descriptor:
-------------------------
0x09 bLength
0x02 bDescriptorType
0x007F wTotalLength (127 bytes)
0x02 bNumInterfaces
0x01 bConfigurationValue
0x00 iConfiguration
0xC0 bmAttributes (Self-powered Device)
0x01 bMaxPower (2 mA)
Interface Association Descriptor:
------------------------------
0x08 bLength
0x0B bDescriptorType
0x00 bFirstInterface
0x02 bInterfaceCount
0x01 bFunctionClass (Audio Device Class)
0x00 bFunctionSubClass
0x20 bFunctionProtocol (Audio Protocol IP version 2.00)
0x04 iFunction "Source/Sink"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x00 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x01 bInterfaceSubClass (Audio Control Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x05 iInterface "Topology Control"
AC Interface Header Descriptor:
------------------------------
0x09 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x0200 bcdADC
0x08 bCategory (IO_BOX)
0x002E wTotalLength (46 bytes)
0x00 bmControls
AC Clock Source Descriptor:
------------------------------
0x08 bLength
0x24 bDescriptorType
0x0A bDescriptorSubtype
0x03 bClockID
0x01 bmAttributes
0x01 bmControls
Clock Frequency Control - read only
0x00 bAssocTerminal
0x06 iClockSource "48000Hz"
AC Input Terminal Descriptor:
------------------------------
0x11 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bTerminalID
0x0200 wTerminalType (Input Undefined)
0x00 bAssocTerminal
0x03 bCSourceID
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
0x03 bmControls
Copy Protect Control - host programmable
0x09 iTerminal "USBD Out"
AC Output Terminal Descriptor:
------------------------------
0x0C bLength
0x24 bDescriptorType
0x03 bDescriptorSubtype
0x02 bTerminalID
0x0101 wTerminalType (USB Streaming)
0x00 bAssocTerminal
0x01 bSourceID
0x03 bCSourceID
0x0003 bmControls
Copy Protect Control - host programmable
0x0A iTerminal "USBH In"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0E iInterface "Capture Inactive"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x01 bAlternateSetting
0x01 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0F iInterface "Capture Active"
AS Interface Descriptor:
------------------------------
0x10 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x02 bTerminalLink
0x00 bmControls
0x01 bFormatType (FORMAT_TYPE_1)
0x00000001 bmFormats
PCM
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
AS Format Type 1 Descriptor:
------------------------------
0x06 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bFormatType (FORMAT_TYPE_1)
0x02 bSubslotSize
0x10 bBitResolution (16 bits per sample)
Endpoint Descriptor:
------------------------------
0x07 bLength
0x05 bDescriptorType
0x81 bEndpointAddress (IN endpoint 1)
0x05 bmAttributes (Transfer: Isochronous / Synch: Asynchronous / Usage: Data)
0x00C0 wMaxPacketSize (1 x 192 bytes)
0x04 bInterval (8 microframes)
AS Isochronous Data Endpoint Descriptor:
------------------------------
0x08 bLength
0x25 bDescriptorType
0x01 bDescriptorSubtype
0x00 bmAttributes
0x00 bmControls
0x00 bLockDelayUnits (undefined)
0x0000 wLockDelay
-------------------------------------
Other Speed Configuration Descriptor:
-------------------------------------
0x09 bLength
0x07 bDescriptorType
0x007F wTotalLength (127 bytes)
0x02 bNumInterfaces
0x01 bConfigurationValue
0x00 iConfiguration
0xC0 bmAttributes (Self-powered Device)
0x01 bMaxPower (2 mA)
Interface Association Descriptor:
------------------------------
0x08 bLength
0x0B bDescriptorType
0x00 bFirstInterface
0x02 bInterfaceCount
0x01 bFunctionClass (Audio Device Class)
0x00 bFunctionSubClass
0x20 bFunctionProtocol (Audio Protocol IP version 2.00)
0x04 iFunction "Source/Sink"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x00 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x01 bInterfaceSubClass (Audio Control Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x05 iInterface "Topology Control"
AC Interface Header Descriptor:
------------------------------
0x09 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x0200 bcdADC
0x08 bCategory (IO_BOX)
0x002E wTotalLength (46 bytes)
0x00 bmControls
AC Clock Source Descriptor:
------------------------------
0x08 bLength
0x24 bDescriptorType
0x0A bDescriptorSubtype
0x03 bClockID
0x01 bmAttributes
0x01 bmControls
Clock Frequency Control - read only
0x00 bAssocTerminal
0x06 iClockSource "48000Hz"
AC Input Terminal Descriptor:
------------------------------
0x11 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bTerminalID
0x0200 wTerminalType (Input Undefined)
0x00 bAssocTerminal
0x03 bCSourceID
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
0x03 bmControls
Copy Protect Control - host programmable
0x09 iTerminal "USBD Out"
AC Output Terminal Descriptor:
------------------------------
0x0C bLength
0x24 bDescriptorType
0x03 bDescriptorSubtype
0x02 bTerminalID
0x0101 wTerminalType (USB Streaming)
0x00 bAssocTerminal
0x01 bSourceID
0x03 bCSourceID
0x0003 bmControls
Copy Protect Control - host programmable
0x0A iTerminal "USBH In"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x00 bAlternateSetting
0x00 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0E iInterface "Capture Inactive"
Interface Descriptor:
------------------------------
0x09 bLength
0x04 bDescriptorType
0x01 bInterfaceNumber
0x01 bAlternateSetting
0x01 bNumEndPoints
0x01 bInterfaceClass (Audio Device Class)
0x02 bInterfaceSubClass (Audio Streaming Interface)
0x20 bInterfaceProtocol (Audio Protocol IP version 2.00)
0x0F iInterface "Capture Active"
AS Interface Descriptor:
------------------------------
0x10 bLength
0x24 bDescriptorType
0x01 bDescriptorSubtype
0x02 bTerminalLink
0x00 bmControls
0x01 bFormatType (FORMAT_TYPE_1)
0x00000001 bmFormats
PCM
0x02 bNrChannels (2 channels)
0x00000003 bmChannelConfig
0x00 iChannelNames
AS Format Type 1 Descriptor:
------------------------------
0x06 bLength
0x24 bDescriptorType
0x02 bDescriptorSubtype
0x01 bFormatType (FORMAT_TYPE_1)
0x02 bSubslotSize
0x10 bBitResolution (16 bits per sample)
Endpoint Descriptor:
------------------------------
0x07 bLength
0x05 bDescriptorType
0x81 bEndpointAddress (IN endpoint 1)
0x05 bmAttributes (Transfer: Isochronous / Synch: Asynchronous / Usage: Data)
0x00C0 wMaxPacketSize (1 x 192 bytes)
0x01 bInterval (1 frames)
AS Isochronous Data Endpoint Descriptor:
------------------------------
0x08 bLength
0x25 bDescriptorType
0x01 bDescriptorSubtype
0x00 bmAttributes
0x00 bmControls
0x00 bLockDelayUnits (undefined)
0x0000 wLockDelay
Microsoft OS Descriptor is not available. Error code: 0x0000001F
--------------------------------
String Descriptor Table
--------------------------------
Index LANGID String
0x00 0x0000 0x0409
0x01 0x0409 "Linux 5.13.1-v8+ with fe980000.usb"
0x02 0x0409 "Linux USB Audio Gadget"
0x04 0x0409 "Source/Sink"
0x05 0x0409 "Topology Control"
0x06 0x0409 "48000Hz"
0x09 0x0409 "USBD Out"
0x0A 0x0409 "USBH In"
0x0E 0x0409 "Capture Inactive"
0x0F 0x0409 "Capture Active"
------------------------------
Connection path for device:
Standard Enhanced PCI to USB Host Controller
Root Hub
Linux USB Audio Gadget (VID=0x1D6B PID=0x0101) Port: 1
Running on: Windows 10 or greater (Build Version 19041)
Brought to you by TDD v2.17.0, Feb 23 2021, 14:04:02
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-13 13:16 ` Pavel Hofman
@ 2021-07-13 13:28 ` Pavel Hofman
2021-07-15 9:39 ` Jerome Brunet
1 sibling, 0 replies; 11+ messages in thread
From: Pavel Hofman @ 2021-07-13 13:28 UTC (permalink / raw)
To: Jerome Brunet, linux-usb; +Cc: Ruslan Bilovol, Felipe Balbi
Dne 13. 07. 21 v 15:16 Pavel Hofman napsal(a):
>
>
> Dne 13. 07. 21 v 14:05 Jerome Brunet napsal(a):
>>
>> On Tue 13 Jul 2021 at 12:22, Pavel Hofman <pavel.hofman@ivitera.com>
>> wrote:
>>
>>> Hi,
>>>
>>> I am testing the three Ruslan's async feedback patches as modified by
>>> Jerome and I hit a regression in windows 10 enumeration.
>>>
>>> Ruslan posted an already accepted patch
>>> https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
>>>
>>> which allowed win10 enumeration.
>>>
>>> Ruslan's async feedback patchset kept the change:
>>> https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
>>>
>>>
>>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>>> b/drivers/usb/gadget/function/f_uac2.c
>>> index 72b42f8..91b22fb 100644
>>> --- a/drivers/usb/gadget/function/f_uac2.c
>>> +++ b/drivers/usb/gadget/function/f_uac2.c
>>> @@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
>>> f_uac2_opts *uac2_opts,
>>>
>>> max_size_bw = num_channels(chmask) * ssize *
>>> ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>>> +
>>> + if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
>>> + max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
>>> +
>>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>>> max_size_ep));
>>>
>>>
>>> Jerome's rebase patch which was accepted recently changed the
>>> functionality
>>> to the original code:
>>> https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
>>>
>>>
>>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>>> b/drivers/usb/gadget/function/f_uac2.c
>>> index 321e6c05ba93..ae29ff2b2b68 100644
>>> --- a/drivers/usb/gadget/function/f_uac2.c
>>> +++ b/drivers/usb/gadget/function/f_uac2.c
>>> @@ -584,8 +584,11 @@ 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;
>>> +
>>> max_size_bw = num_channels(chmask) * ssize *
>>> - ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>>> + DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
>>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>>> max_size_ep));
>>>
>>> With this version my Win10 does not enumerate the USB AUDIO device,
>>> even if
>>> it has only EP-IN capability (i.e. is_playback = true). For my setup the
>>> EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version,
>>> causing
>>> win10 reporting "The specified range could not be found in the range
>>> list."
>>>
>>
>> Maybe I am lacking USB expertize, but is there any reason why a 192bytes
>> maximum packet size should be considered invalid ? Just from your
>> comment, I can't figure it out.
>>
>> It would help if you could detail the parameters you started your gadget
>> with (rate, format, channel number)
>>
>> IIRC, Ruslan initial patchset reserved a fixed additional bandwidth
>> (around 20% I think) to deal with the explicit feedback.
>>
>> The idea is that, 99.9% of the time, all you need is the ability to fit
>> one more sample in each packet. This is should be what the default
>> setting provides (up to 192kHz). If it does not do that, I made mistake.
>>
>> The setting configurable because I was trying to avoid wasting USB
>> bandwith but still support poor quality platforms where 1 extra sample
>> is not enough (I think Ruslan mentioned having seen such system)
>
> I am absolutely no USB expert. What I did was testing Jerome's patchset
> and win10 refused to enumerate, even with the most trivial configuration
> c_srate=p_srate=48000, 16bits, capture 2ch, playback 0ch which
> configures no EP-OUT and no feedback EP-IN. To find the cause I went
> back one patch (HEAD^) and win10 happily enumerated this no-EP-OUT
> configuration. So I compared the usb config dump - see attached files
> from Theosycon USB Descriptor Dumper - see the two attached files, named
> after commit IDs.
>
> The dumps differ in only one parameter EP-IN 1 wMaxPacketSize, for both
> HS and the dumped "Other Speed Configuration Descriptor" i.e. FS:
>
> diff 650f7f40dc1691a8ab4d1dc411f8f327b36e8d14.txt
> cb1c270600b2c6f55f55f227775aaddf2cc78bed.txt
> 185c185
> < 0x00C4 wMaxPacketSize (1 x 196 bytes)
> ---
> > 0x00C0 wMaxPacketSize (1 x 192 bytes)
> 339c339
> < 0x00C4 wMaxPacketSize (1 x 196 bytes)
> ---
> > 0x00C0 wMaxPacketSize (1 x 192 bytes)
>
> So I looked at the patch and saw the changed wMaxPacketSize calculation.
> Adding +1 yielded what the original Ruslan's code yielded - one more
> sample (i.e. 196 bytes instead of 192bytes). Therefore I put it in the
> patch here. This value is accepted by win10.
>
> I do not know how windows uac2 driver verifies validity of
> wMaxPacketSize but clearly 196bytes (+1 sample) is accepted while
> 192bytes is refused. Linux does not care, just like Ruslan described in
> his commit 789ea77310f020.
>
> I checked duplex with EP-OUT enabled (i.e. with the feedback EP-IN), the
> "new" calculation works OK in win10 (while the Jerome's values were
> refused).
>
> I really do not know the correct code for calculating the wMaxPacketSize
> to satisfy the windows driver, but what I am posting works. I'll be
> happy if someone knowledgeable fixes my layman quickfix.
>
The original Ruslan's patch (in the mainline branch
https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a
) explains the reasoning.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-13 13:16 ` Pavel Hofman
2021-07-13 13:28 ` Pavel Hofman
@ 2021-07-15 9:39 ` Jerome Brunet
2021-07-15 12:36 ` Pavel Hofman
1 sibling, 1 reply; 11+ messages in thread
From: Jerome Brunet @ 2021-07-15 9:39 UTC (permalink / raw)
To: Pavel Hofman, linux-usb; +Cc: Ruslan Bilovol, Felipe Balbi
On Tue 13 Jul 2021 at 15:16, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
> Dne 13. 07. 21 v 14:05 Jerome Brunet napsal(a):
>>
>> On Tue 13 Jul 2021 at 12:22, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>>
>>> Hi,
>>>
>>> I am testing the three Ruslan's async feedback patches as modified by
>>> Jerome and I hit a regression in windows 10 enumeration.
>>>
>>> Ruslan posted an already accepted patch
>>> https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
>>> which allowed win10 enumeration.
>>>
>>> Ruslan's async feedback patchset kept the change:
>>> https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
>>>
>>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>>> b/drivers/usb/gadget/function/f_uac2.c
>>> index 72b42f8..91b22fb 100644
>>> --- a/drivers/usb/gadget/function/f_uac2.c
>>> +++ b/drivers/usb/gadget/function/f_uac2.c
>>> @@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
>>> f_uac2_opts *uac2_opts,
>>>
>>> max_size_bw = num_channels(chmask) * ssize *
>>> ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>>> +
>>> + if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
>>> + max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
>>> +
>>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>>> max_size_ep));
>>>
>>>
>>> Jerome's rebase patch which was accepted recently changed the functionality
>>> to the original code:
>>> https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
>>>
>>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>>> b/drivers/usb/gadget/function/f_uac2.c
>>> index 321e6c05ba93..ae29ff2b2b68 100644
>>> --- a/drivers/usb/gadget/function/f_uac2.c
>>> +++ b/drivers/usb/gadget/function/f_uac2.c
>>> @@ -584,8 +584,11 @@ 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;
>>> +
>>> max_size_bw = num_channels(chmask) * ssize *
>>> - ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>>> + DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
>>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>>> max_size_ep));
>>>
>>> With this version my Win10 does not enumerate the USB AUDIO device, even if
>>> it has only EP-IN capability (i.e. is_playback = true). For my setup the
>>> EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version, causing
>>> win10 reporting "The specified range could not be found in the range list."
>>>
>>
>> Maybe I am lacking USB expertize, but is there any reason why a 192bytes
>> maximum packet size should be considered invalid ? Just from your
>> comment, I can't figure it out.
>>
>> It would help if you could detail the parameters you started your gadget
>> with (rate, format, channel number)
>>
>> IIRC, Ruslan initial patchset reserved a fixed additional bandwidth
>> (around 20% I think) to deal with the explicit feedback.
>>
>> The idea is that, 99.9% of the time, all you need is the ability to fit
>> one more sample in each packet. This is should be what the default
>> setting provides (up to 192kHz). If it does not do that, I made mistake.
>>
>> The setting configurable because I was trying to avoid wasting USB
>> bandwith but still support poor quality platforms where 1 extra sample
>> is not enough (I think Ruslan mentioned having seen such system)
>
> I am absolutely no USB expert. What I did was testing Jerome's patchset
> and win10 refused to enumerate, even with the most trivial configuration
> c_srate=p_srate=48000, 16bits, capture 2ch, playback 0ch which
> configures no EP-OUT and no feedback EP-IN. To find the cause I went
> back one patch (HEAD^) and win10 happily enumerated this no-EP-OUT
> configuration. So I compared the usb config dump - see attached files
> from Theosycon USB Descriptor Dumper - see the two attached files, named
> after commit IDs.
So 48kHz / 2ch / 16bits. Let's assume USB_SPEED_FULL for example (result
is the same for the other speeds).
In such condition, the nominal packet size is 192B but to accomodate an
extra sample, the maximum should indeed be 196B.
if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
srate = srate * (1000 + uac2_opts->fb_max) / 1000;
with fb_max being 5 by default, srate should be 48240 after this.
max_size_bw = num_channels(chmask) * ssize *
DIV_ROUND_UP(srate, factor / (1 << (bInterval - 1)));
With USB_SPEED_FULL, bInterval is 1 and factor is 1000 so:
=> DIV_ROUND_UP(48240, 1000 / 1) should give 49;
Then
=> max_size_bw = 2 * 2 * 49 = 196
So the end result should be 196 with current code. I tried on an ARM64
platform. Here is what I get:
[ 26.241946] set_ep_max_packet_size: speed is USB_SPEED_FULL
[ 26.243208] set_ep_max_packet_size: intermediate Playback srate 48000
[ 26.249758] set_ep_max_packet_size: max_size_bw 192
[ 26.254559] set_ep_max_packet_size: speed is USB_SPEED_FULL
[ 26.260130] set_ep_max_packet_size: intermediate Capture srate 48240
[ 26.266401] set_ep_max_packet_size: max_size_bw 196
[ 26.271209] set_ep_max_packet_size: speed is USB_SPEED_HIGH
[ 26.276873] set_ep_max_packet_size: intermediate Playback srate 48000
[ 26.283165] set_ep_max_packet_size: max_size_bw 192
[ 26.288015] set_ep_max_packet_size: speed is USB_SPEED_HIGH
[ 26.293691] set_ep_max_packet_size: intermediate Capture srate 48240
[ 26.299965] set_ep_max_packet_size: max_size_bw 196
[ 26.304753] set_ep_max_packet_size: speed is USB_SPEED_SUPER
[ 26.310426] set_ep_max_packet_size: intermediate Playback srate 48000
[ 26.316805] set_ep_max_packet_size: max_size_bw 192
[ 26.321625] set_ep_max_packet_size: speed is USB_SPEED_SUPER
[ 26.327309] set_ep_max_packet_size: intermediate Capture srate 48240
[ 26.333613] set_ep_max_packet_size: max_size_bw 196
All seems OK and as expected with what's in mainline ATM.
So I'm not quite sure why you would get a different result. It would be
great if you could check further.
>
> The dumps differ in only one parameter EP-IN 1 wMaxPacketSize, for both
> HS and the dumped "Other Speed Configuration Descriptor" i.e. FS:
>
> diff 650f7f40dc1691a8ab4d1dc411f8f327b36e8d14.txt
> cb1c270600b2c6f55f55f227775aaddf2cc78bed.txt
> 185c185
> < 0x00C4 wMaxPacketSize (1 x 196 bytes)
> ---
> > 0x00C0 wMaxPacketSize (1 x 192 bytes)
> 339c339
> < 0x00C4 wMaxPacketSize (1 x 196 bytes)
> ---
> > 0x00C0 wMaxPacketSize (1 x 192 bytes)
>
> So I looked at the patch and saw the changed wMaxPacketSize calculation.
> Adding +1 yielded what the original Ruslan's code yielded - one more
> sample (i.e. 196 bytes instead of 192bytes). Therefore I put it in the
> patch here. This value is accepted by win10.
>
> I do not know how windows uac2 driver verifies validity of
> wMaxPacketSize but clearly 196bytes (+1 sample) is accepted while
> 192bytes is refused. Linux does not care, just like Ruslan described in
> his commit 789ea77310f020.
>
> I checked duplex with EP-OUT enabled (i.e. with the feedback EP-IN), the
> "new" calculation works OK in win10 (while the Jerome's values were
> refused).
>
> I really do not know the correct code for calculating the wMaxPacketSize
> to satisfy the windows driver, but what I am posting works. I'll be
> happy if someone knowledgeable fixes my layman quickfix.
>
> Best regards,
>
> Pavel.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-15 9:39 ` Jerome Brunet
@ 2021-07-15 12:36 ` Pavel Hofman
2021-07-15 13:53 ` Jerome Brunet
0 siblings, 1 reply; 11+ messages in thread
From: Pavel Hofman @ 2021-07-15 12:36 UTC (permalink / raw)
To: Jerome Brunet, linux-usb; +Cc: Ruslan Bilovol, Felipe Balbi
Dne 15. 07. 21 v 11:39 Jerome Brunet napsal(a):
>
> On Tue 13 Jul 2021 at 15:16, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>
>> Dne 13. 07. 21 v 14:05 Jerome Brunet napsal(a):
>>>
>>> On Tue 13 Jul 2021 at 12:22, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> I am testing the three Ruslan's async feedback patches as modified by
>>>> Jerome and I hit a regression in windows 10 enumeration.
>>>>
>>>> Ruslan posted an already accepted patch
>>>> https://github.com/torvalds/linux/commit/789ea77310f0200c84002884ffd628e2baf3ad8a#diff-876615ece7fb56ce8d45fc70623bef9caa2548e810426f217fb785ffa10b33d5
>>>> which allowed win10 enumeration.
>>>>
>>>> Ruslan's async feedback patchset kept the change:
>>>> https://patchwork.kernel.org/project/linux-usb/patch/1614603943-11668-5-git-send-email-ruslan.bilovol@gmail.com/
>>>>
>>>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>>>> b/drivers/usb/gadget/function/f_uac2.c
>>>> index 72b42f8..91b22fb 100644
>>>> --- a/drivers/usb/gadget/function/f_uac2.c
>>>> +++ b/drivers/usb/gadget/function/f_uac2.c
>>>> @@ -506,6 +506,10 @@ static int set_ep_max_packet_size(const struct
>>>> f_uac2_opts *uac2_opts,
>>>>
>>>> max_size_bw = num_channels(chmask) * ssize *
>>>> ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>>>> +
>>>> + if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
>>>> + max_size_bw = max_size_bw * FBACK_FREQ_MAX / 100;
>>>> +
>>>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>>>> max_size_ep));
>>>>
>>>>
>>>> Jerome's rebase patch which was accepted recently changed the functionality
>>>> to the original code:
>>>> https://patchwork.kernel.org/project/linux-usb/patch/20210603220104.1216001-4-jbrunet@baylibre.com/
>>>>
>>>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>>>> b/drivers/usb/gadget/function/f_uac2.c
>>>> index 321e6c05ba93..ae29ff2b2b68 100644
>>>> --- a/drivers/usb/gadget/function/f_uac2.c
>>>> +++ b/drivers/usb/gadget/function/f_uac2.c
>>>> @@ -584,8 +584,11 @@ 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;
>>>> +
>>>> max_size_bw = num_channels(chmask) * ssize *
>>>> - ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>>>> + DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
>>>> ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
>>>> max_size_ep));
>>>>
>>>> With this version my Win10 does not enumerate the USB AUDIO device, even if
>>>> it has only EP-IN capability (i.e. is_playback = true). For my setup the
>>>> EP-IN wMaxPacketSize is 192bytes vs. 196bytes in Ruslan's version, causing
>>>> win10 reporting "The specified range could not be found in the range list."
>>>>
>>>
>>> Maybe I am lacking USB expertize, but is there any reason why a 192bytes
>>> maximum packet size should be considered invalid ? Just from your
>>> comment, I can't figure it out.
>>>
>>> It would help if you could detail the parameters you started your gadget
>>> with (rate, format, channel number)
>>>
>>> IIRC, Ruslan initial patchset reserved a fixed additional bandwidth
>>> (around 20% I think) to deal with the explicit feedback.
>>>
>>> The idea is that, 99.9% of the time, all you need is the ability to fit
>>> one more sample in each packet. This is should be what the default
>>> setting provides (up to 192kHz). If it does not do that, I made mistake.
>>>
>>> The setting configurable because I was trying to avoid wasting USB
>>> bandwith but still support poor quality platforms where 1 extra sample
>>> is not enough (I think Ruslan mentioned having seen such system)
>>
>> I am absolutely no USB expert. What I did was testing Jerome's patchset
>> and win10 refused to enumerate, even with the most trivial configuration
>> c_srate=p_srate=48000, 16bits, capture 2ch, playback 0ch which
>> configures no EP-OUT and no feedback EP-IN. To find the cause I went
>> back one patch (HEAD^) and win10 happily enumerated this no-EP-OUT
>> configuration. So I compared the usb config dump - see attached files
>> from Theosycon USB Descriptor Dumper - see the two attached files, named
>> after commit IDs.
>
> So 48kHz / 2ch / 16bits. Let's assume USB_SPEED_FULL for example (result
> is the same for the other speeds).
>
> In such condition, the nominal packet size is 192B but to accomodate an
> extra sample, the maximum should indeed be 196B.
>
> if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> srate = srate * (1000 + uac2_opts->fb_max) / 1000;
>
> with fb_max being 5 by default, srate should be 48240 after this.
>
>
> max_size_bw = num_channels(chmask) * ssize *
> DIV_ROUND_UP(srate, factor / (1 << (bInterval - 1)));
>
> With USB_SPEED_FULL, bInterval is 1 and factor is 1000 so:
> => DIV_ROUND_UP(48240, 1000 / 1) should give 49;
>
> Then
> => max_size_bw = 2 * 2 * 49 = 196
>
> So the end result should be 196 with current code. I tried on an ARM64
> platform. Here is what I get:
>
> [ 26.241946] set_ep_max_packet_size: speed is USB_SPEED_FULL
> [ 26.243208] set_ep_max_packet_size: intermediate Playback srate 48000
> [ 26.249758] set_ep_max_packet_size: max_size_bw 192
> [ 26.254559] set_ep_max_packet_size: speed is USB_SPEED_FULL
> [ 26.260130] set_ep_max_packet_size: intermediate Capture srate 48240
> [ 26.266401] set_ep_max_packet_size: max_size_bw 196
> [ 26.271209] set_ep_max_packet_size: speed is USB_SPEED_HIGH
> [ 26.276873] set_ep_max_packet_size: intermediate Playback srate 48000
> [ 26.283165] set_ep_max_packet_size: max_size_bw 192
> [ 26.288015] set_ep_max_packet_size: speed is USB_SPEED_HIGH
> [ 26.293691] set_ep_max_packet_size: intermediate Capture srate 48240
> [ 26.299965] set_ep_max_packet_size: max_size_bw 196
> [ 26.304753] set_ep_max_packet_size: speed is USB_SPEED_SUPER
> [ 26.310426] set_ep_max_packet_size: intermediate Playback srate 48000
> [ 26.316805] set_ep_max_packet_size: max_size_bw 192
> [ 26.321625] set_ep_max_packet_size: speed is USB_SPEED_SUPER
> [ 26.327309] set_ep_max_packet_size: intermediate Capture srate 48240
> [ 26.333613] set_ep_max_packet_size: max_size_bw 196
>
> All seems OK and as expected with what's in mainline ATM.
> So I'm not quite sure why you would get a different result. It would be
> great if you could check further.
>
The problem is max_size_bw=192 for the Playback (i.e. is_playback =
true). If only capture direction is activated (p_chmask=0), only EP-OUT
with max_size_bw=196 is generated and Win10 enumerates the playback-only
audio device. Once the other direction with max_size_bw=192 is activated
(either duplex or capture-only), Win10 refuses to enumerate.
This generates 196bytes for each direction and works OK in Win10:
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index ae29ff2b2b68..f6ccc21972bf 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -584,11 +584,15 @@ 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))
+ if (!is_playback && (uac2_opts->c_sync ==
USB_ENDPOINT_SYNC_ASYNC)) {
srate = srate * (1000 + uac2_opts->fb_max) / 1000;
+ max_size_bw = num_channels(chmask) * ssize *
+ DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
+ } else {
+ max_size_bw = num_channels(chmask) * ssize *
+ (DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)))
+ 1);
+ }
- max_size_bw = num_channels(chmask) * ssize *
- DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval -
1)));
ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
max_size_ep));
Tested for samplerate 250kHz 2bytes each direction yielding 1008bytes
for EP-OUT and 1004 bytes for EP-IN - Win10 enumerates OK.
Pavel.
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-15 12:36 ` Pavel Hofman
@ 2021-07-15 13:53 ` Jerome Brunet
2021-07-15 14:22 ` Pavel Hofman
2021-07-15 17:52 ` Jassi Brar
0 siblings, 2 replies; 11+ messages in thread
From: Jerome Brunet @ 2021-07-15 13:53 UTC (permalink / raw)
To: Pavel Hofman, linux-usb; +Cc: Ruslan Bilovol, Felipe Balbi
On Thu 15 Jul 2021 at 14:36, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
> Dne 15. 07. 21 v 11:39 Jerome Brunet napsal(a):
>> On Tue 13 Jul 2021 at 15:16, Pavel Hofman <pavel.hofman@ivitera.com>
>> wrote:
>>
>> So 48kHz / 2ch / 16bits. Let's assume USB_SPEED_FULL for example (result
>> is the same for the other speeds).
>> In such condition, the nominal packet size is 192B but to accomodate an
>> extra sample, the maximum should indeed be 196B.
>> if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
>> srate = srate * (1000 + uac2_opts->fb_max) / 1000;
>> with fb_max being 5 by default, srate should be 48240 after this.
>>
>> max_size_bw = num_channels(chmask) * ssize *
>> DIV_ROUND_UP(srate, factor / (1 << (bInterval - 1)));
>> With USB_SPEED_FULL, bInterval is 1 and factor is 1000 so:
>> => DIV_ROUND_UP(48240, 1000 / 1) should give 49;
>> Then
>> => max_size_bw = 2 * 2 * 49 = 196
>> So the end result should be 196 with current code. I tried on an ARM64
>> platform. Here is what I get:
>> [ 26.241946] set_ep_max_packet_size: speed is USB_SPEED_FULL
>> [ 26.243208] set_ep_max_packet_size: intermediate Playback srate 48000
>> [ 26.249758] set_ep_max_packet_size: max_size_bw 192
>> [ 26.254559] set_ep_max_packet_size: speed is USB_SPEED_FULL
>> [ 26.260130] set_ep_max_packet_size: intermediate Capture srate 48240
>> [ 26.266401] set_ep_max_packet_size: max_size_bw 196
>> [ 26.271209] set_ep_max_packet_size: speed is USB_SPEED_HIGH
>> [ 26.276873] set_ep_max_packet_size: intermediate Playback srate 48000
>> [ 26.283165] set_ep_max_packet_size: max_size_bw 192
>> [ 26.288015] set_ep_max_packet_size: speed is USB_SPEED_HIGH
>> [ 26.293691] set_ep_max_packet_size: intermediate Capture srate 48240
>> [ 26.299965] set_ep_max_packet_size: max_size_bw 196
>> [ 26.304753] set_ep_max_packet_size: speed is USB_SPEED_SUPER
>> [ 26.310426] set_ep_max_packet_size: intermediate Playback srate 48000
>> [ 26.316805] set_ep_max_packet_size: max_size_bw 192
>> [ 26.321625] set_ep_max_packet_size: speed is USB_SPEED_SUPER
>> [ 26.327309] set_ep_max_packet_size: intermediate Capture srate 48240
>> [ 26.333613] set_ep_max_packet_size: max_size_bw 196
>> All seems OK and as expected with what's in mainline ATM.
>> So I'm not quite sure why you would get a different result. It would be
>> great if you could check further.
>>
>
> The problem is max_size_bw=192 for the Playback (i.e. is_playback =
> true). If only capture direction is activated (p_chmask=0), only EP-OUT
> with max_size_bw=196 is generated and Win10 enumerates the playback-only
> audio device.
Ok, that was not clear before.
> Once the other direction with max_size_bw=192 is activated
> (either duplex or capture-only), Win10 refuses to enumerate.
Looking further at the format specification [0] (and crawling the web to
decipher it), it seems that
* For isochronous links: packet size must match the nominal rate.
* For async and adaptative: it must match the nominal rate +/- 1
packet. That is whether we intend on varying the packet size or not.
This has several implication
* In async mode, the device is running of its own clock. It has no
reason to vary the playback packet size but it should still reserve
bandwidth for an extra packet to satisfy the spec. This seems to be
your problem and what Win10 insist on.
When I tested, I had linux on both sides and apparently it is not too
picky about that.
* If we apply the spec strictly, like Win10 seems to insist on,
calculating the maximum packet size based on explicit feedback limits
is wrong too. Whatever happens, it should be +/- 1 around nominal.
Funny thing, is your change puts a +2 capture compared to nominal but
Win10 is not picky on that ...
I'll send a fix to clean this up. Thanks reporting the problem.
[0]: https://www.usb.org/sites/default/files/frmts10.pdf
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-15 13:53 ` Jerome Brunet
@ 2021-07-15 14:22 ` Pavel Hofman
2021-07-15 17:52 ` Jassi Brar
1 sibling, 0 replies; 11+ messages in thread
From: Pavel Hofman @ 2021-07-15 14:22 UTC (permalink / raw)
To: Jerome Brunet, linux-usb; +Cc: Ruslan Bilovol, Felipe Balbi
Dne 15. 07. 21 v 15:53 Jerome Brunet napsal(a):
>
> On Thu 15 Jul 2021 at 14:36, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>
>> Dne 15. 07. 21 v 11:39 Jerome Brunet napsal(a):
>>> On Tue 13 Jul 2021 at 15:16, Pavel Hofman <pavel.hofman@ivitera.com>
>>> wrote:
>>>
>
>>> So 48kHz / 2ch / 16bits. Let's assume USB_SPEED_FULL for example (result
>>> is the same for the other speeds).
>>> In such condition, the nominal packet size is 192B but to accomodate an
>>> extra sample, the maximum should indeed be 196B.
>>> if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
>>> srate = srate * (1000 + uac2_opts->fb_max) / 1000;
>>> with fb_max being 5 by default, srate should be 48240 after this.
>>>
>>> max_size_bw = num_channels(chmask) * ssize *
>>> DIV_ROUND_UP(srate, factor / (1 << (bInterval - 1)));
>>> With USB_SPEED_FULL, bInterval is 1 and factor is 1000 so:
>>> => DIV_ROUND_UP(48240, 1000 / 1) should give 49;
>>> Then
>>> => max_size_bw = 2 * 2 * 49 = 196
>>> So the end result should be 196 with current code. I tried on an ARM64
>>> platform. Here is what I get:
>>> [ 26.241946] set_ep_max_packet_size: speed is USB_SPEED_FULL
>>> [ 26.243208] set_ep_max_packet_size: intermediate Playback srate 48000
>>> [ 26.249758] set_ep_max_packet_size: max_size_bw 192
>>> [ 26.254559] set_ep_max_packet_size: speed is USB_SPEED_FULL
>>> [ 26.260130] set_ep_max_packet_size: intermediate Capture srate 48240
>>> [ 26.266401] set_ep_max_packet_size: max_size_bw 196
>>> [ 26.271209] set_ep_max_packet_size: speed is USB_SPEED_HIGH
>>> [ 26.276873] set_ep_max_packet_size: intermediate Playback srate 48000
>>> [ 26.283165] set_ep_max_packet_size: max_size_bw 192
>>> [ 26.288015] set_ep_max_packet_size: speed is USB_SPEED_HIGH
>>> [ 26.293691] set_ep_max_packet_size: intermediate Capture srate 48240
>>> [ 26.299965] set_ep_max_packet_size: max_size_bw 196
>>> [ 26.304753] set_ep_max_packet_size: speed is USB_SPEED_SUPER
>>> [ 26.310426] set_ep_max_packet_size: intermediate Playback srate 48000
>>> [ 26.316805] set_ep_max_packet_size: max_size_bw 192
>>> [ 26.321625] set_ep_max_packet_size: speed is USB_SPEED_SUPER
>>> [ 26.327309] set_ep_max_packet_size: intermediate Capture srate 48240
>>> [ 26.333613] set_ep_max_packet_size: max_size_bw 196
>>> All seems OK and as expected with what's in mainline ATM.
>>> So I'm not quite sure why you would get a different result. It would be
>>> great if you could check further.
>>>
>>
>> The problem is max_size_bw=192 for the Playback (i.e. is_playback =
>> true). If only capture direction is activated (p_chmask=0), only EP-OUT
>> with max_size_bw=196 is generated and Win10 enumerates the playback-only
>> audio device.
>
> Ok, that was not clear before.
>
>> Once the other direction with max_size_bw=192 is activated
>> (either duplex or capture-only), Win10 refuses to enumerate.
>
> Looking further at the format specification [0] (and crawling the web to
> decipher it), it seems that
>
> * For isochronous links: packet size must match the nominal rate.
> * For async and adaptative: it must match the nominal rate +/- 1
> packet. That is whether we intend on varying the packet size or not.
>
> This has several implication
> * In async mode, the device is running of its own clock. It has no
> reason to vary the playback packet size but it should still reserve
> bandwidth for an extra packet to satisfy the spec. This seems to be
> your problem and what Win10 insist on.
>
> When I tested, I had linux on both sides and apparently it is not too
> picky about that.
Linux also accepts the async EP-OUT without the explicit feedback EP-IN,
unlike Win10. It also supports implicit async feedback, unlike Win10.
But the WASAPI usbaudio2.sys driver in Win10 seems quite OK, I just
tested 1536kHz/2ch/16bits playback and works OK, also non-standard
samplerates are supported (I tested 512kHz/2ch/32bit), all with overhead
of the Win10 running in VMWare Player virtualization.
>
> * If we apply the spec strictly, like Win10 seems to insist on,
> calculating the maximum packet size based on explicit feedback limits
> is wrong too. Whatever happens, it should be +/- 1 around nominal.
>
> Funny thing, is your change puts a +2 capture compared to nominal but
> Win10 is not picky on that ...
IMO the Win10 driver just makes sure the max packet size is at least the
maximum in the specs. If it's more, it does not care - more USB
bandwidth is wasted, but the operation is safe.
>
> I'll send a fix to clean this up. Thanks reporting the problem.
Thanks a lot,
Pavel.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation
2021-07-15 13:53 ` Jerome Brunet
2021-07-15 14:22 ` Pavel Hofman
@ 2021-07-15 17:52 ` Jassi Brar
1 sibling, 0 replies; 11+ messages in thread
From: Jassi Brar @ 2021-07-15 17:52 UTC (permalink / raw)
To: Jerome Brunet; +Cc: Pavel Hofman, linux-usb, Ruslan Bilovol, Felipe Balbi
On Thu, Jul 15, 2021 at 12:40 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Thu 15 Jul 2021 at 14:36, Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>
> > Dne 15. 07. 21 v 11:39 Jerome Brunet napsal(a):
> >> On Tue 13 Jul 2021 at 15:16, Pavel Hofman <pavel.hofman@ivitera.com>
> >> wrote:
> >>
>
> >> So 48kHz / 2ch / 16bits. Let's assume USB_SPEED_FULL for example (result
> >> is the same for the other speeds).
> >> In such condition, the nominal packet size is 192B but to accomodate an
> >> extra sample, the maximum should indeed be 196B.
> >> if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> >> srate = srate * (1000 + uac2_opts->fb_max) / 1000;
> >> with fb_max being 5 by default, srate should be 48240 after this.
> >>
> >> max_size_bw = num_channels(chmask) * ssize *
> >> DIV_ROUND_UP(srate, factor / (1 << (bInterval - 1)));
> >> With USB_SPEED_FULL, bInterval is 1 and factor is 1000 so:
> >> => DIV_ROUND_UP(48240, 1000 / 1) should give 49;
> >> Then
> >> => max_size_bw = 2 * 2 * 49 = 196
> >> So the end result should be 196 with current code. I tried on an ARM64
> >> platform. Here is what I get:
> >> [ 26.241946] set_ep_max_packet_size: speed is USB_SPEED_FULL
> >> [ 26.243208] set_ep_max_packet_size: intermediate Playback srate 48000
> >> [ 26.249758] set_ep_max_packet_size: max_size_bw 192
> >> [ 26.254559] set_ep_max_packet_size: speed is USB_SPEED_FULL
> >> [ 26.260130] set_ep_max_packet_size: intermediate Capture srate 48240
> >> [ 26.266401] set_ep_max_packet_size: max_size_bw 196
> >> [ 26.271209] set_ep_max_packet_size: speed is USB_SPEED_HIGH
> >> [ 26.276873] set_ep_max_packet_size: intermediate Playback srate 48000
> >> [ 26.283165] set_ep_max_packet_size: max_size_bw 192
> >> [ 26.288015] set_ep_max_packet_size: speed is USB_SPEED_HIGH
> >> [ 26.293691] set_ep_max_packet_size: intermediate Capture srate 48240
> >> [ 26.299965] set_ep_max_packet_size: max_size_bw 196
> >> [ 26.304753] set_ep_max_packet_size: speed is USB_SPEED_SUPER
> >> [ 26.310426] set_ep_max_packet_size: intermediate Playback srate 48000
> >> [ 26.316805] set_ep_max_packet_size: max_size_bw 192
> >> [ 26.321625] set_ep_max_packet_size: speed is USB_SPEED_SUPER
> >> [ 26.327309] set_ep_max_packet_size: intermediate Capture srate 48240
> >> [ 26.333613] set_ep_max_packet_size: max_size_bw 196
> >> All seems OK and as expected with what's in mainline ATM.
> >> So I'm not quite sure why you would get a different result. It would be
> >> great if you could check further.
> >>
> >
> > The problem is max_size_bw=192 for the Playback (i.e. is_playback =
> > true). If only capture direction is activated (p_chmask=0), only EP-OUT
> > with max_size_bw=196 is generated and Win10 enumerates the playback-only
> > audio device.
>
> Ok, that was not clear before.
>
> > Once the other direction with max_size_bw=192 is activated
> > (either duplex or capture-only), Win10 refuses to enumerate.
>
> Looking further at the format specification [0] (and crawling the web to
> decipher it), it seems that
>
> * For isochronous links: packet size must match the nominal rate.
> * For async and adaptative: it must match the nominal rate +/- 1
> packet. That is whether we intend on varying the packet size or not.
>
> This has several implication
> * In async mode, the device is running of its own clock. It has no
> reason to vary the playback packet size but it should still reserve
> bandwidth for an extra packet to satisfy the spec. This seems to be
> your problem and what Win10 insist on.
>
> When I tested, I had linux on both sides and apparently it is not too
> picky about that.
>
> * If we apply the spec strictly, like Win10 seems to insist on,
> calculating the maximum packet size based on explicit feedback limits
> is wrong too. Whatever happens, it should be +/- 1 around nominal.
>
IIRC, a couple of years ago, Alan proposed a smart scheme to keep the
UAC2 byterate in exact sync with what's expected over time. You may
have to dig into the alsa archives though.
cheers.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-07-15 17:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 10:22 usb:gadget:u_audio: Regression in [v3,3/3] usb: gadget: u_audio: add real feedback implementation - wMaxPacketSize calculation Pavel Hofman
2021-07-13 10:49 ` Greg KH
2021-07-13 12:05 ` Jerome Brunet
2021-07-13 12:32 ` Felipe Balbi
2021-07-13 13:16 ` Pavel Hofman
2021-07-13 13:28 ` Pavel Hofman
2021-07-15 9:39 ` Jerome Brunet
2021-07-15 12:36 ` Pavel Hofman
2021-07-15 13:53 ` Jerome Brunet
2021-07-15 14:22 ` Pavel Hofman
2021-07-15 17:52 ` Jassi Brar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.