| Message ID | 20210924080027.5362-1-pavel.hofman@ivitera.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: f_uac2: fixed EP-IN wMaxPacketSize | expand |
On Fri, Sep 24, 2021 at 10:00:27AM +0200, Pavel Hofman wrote: > Async feedback patches broke enumeration on Windows 10 previously fixed > by commit 789ea77310f0 ("usb: gadget: f_uac2: always increase endpoint > max_packet_size by one audio slot"). > > While the existing calculation for EP OUT capture for async mode yields > size+1 frame due to uac2_opts->fb_max > 0, playback side lost the +1 > feature. Therefore the +1 frame addition must be re-introduced for > playback. Win10 enumerates the device only when both EP IN and EP OUT > max packet sizes are (at least) +1 frame. > > Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com> > Tested-by: Henrik Enquist <henrik.enquist@gmail.com> > Tested-by: Jack Pham <jackp@codeaurora.org> > --- > drivers/usb/gadget/function/f_uac2.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) What commit does this fix? Should it go to stable kernel(s)? thanks, greg k-h
Hi Greg, Dne 24. 09. 21 v 10:20 Greg KH napsal(a): > On Fri, Sep 24, 2021 at 10:00:27AM +0200, Pavel Hofman wrote: >> Async feedback patches broke enumeration on Windows 10 previously fixed >> by commit 789ea77310f0 ("usb: gadget: f_uac2: always increase endpoint >> max_packet_size by one audio slot"). >> >> While the existing calculation for EP OUT capture for async mode yields >> size+1 frame due to uac2_opts->fb_max > 0, playback side lost the +1 >> feature. Therefore the +1 frame addition must be re-introduced for >> playback. Win10 enumerates the device only when both EP IN and EP OUT >> max packet sizes are (at least) +1 frame. >> >> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com> >> Tested-by: Henrik Enquist <henrik.enquist@gmail.com> >> Tested-by: Jack Pham <jackp@codeaurora.org> >> --- >> drivers/usb/gadget/function/f_uac2.c | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) > > What commit does this fix? > > Should it go to stable kernel(s)? It's the same effort direction as commit https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/usb/+/f5dfd98a80ff8d50cf4ae2820857d7f5a46cbab9 which you added to your usb-linus branch https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/usb/+log/refs/heads/usb-linus/drivers/usb/gadget/function Therefore I would say usb-linus. Please what are your rules for usb-linus and usb-next? There are some different commits in usb-next and usb-linus changing the same file - f_uac2.c . Likely not, but a conflict could potentially occur. I do not know what head to rebase my further patches which also make larger changes to that file. Thanks a lot for explanation, Pavel.
Hi Pavel, Thanks for sending out the patch. On Fri, Sep 24, 2021 at 10:35:07AM +0200, Pavel Hofman wrote: > Hi Greg, > > Dne 24. 09. 21 v 10:20 Greg KH napsal(a): > > On Fri, Sep 24, 2021 at 10:00:27AM +0200, Pavel Hofman wrote: > > > Async feedback patches broke enumeration on Windows 10 previously fixed > > > by commit 789ea77310f0 ("usb: gadget: f_uac2: always increase endpoint > > > max_packet_size by one audio slot"). > > > > > > While the existing calculation for EP OUT capture for async mode yields > > > size+1 frame due to uac2_opts->fb_max > 0, playback side lost the +1 > > > feature. Therefore the +1 frame addition must be re-introduced for > > > playback. Win10 enumerates the device only when both EP IN and EP OUT > > > max packet sizes are (at least) +1 frame. > > > > > > Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com> > > > Tested-by: Henrik Enquist <henrik.enquist@gmail.com> > > > Tested-by: Jack Pham <jackp@codeaurora.org> > > > --- > > > drivers/usb/gadget/function/f_uac2.c | 14 ++++++++++---- > > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > What commit does this fix? IMO it Fixes: e89bb4288378 ("usb: gadget: u_audio: add real feedback implementation"). > > Should it go to stable kernel(s)? Yes, but probably just to 5.14 since the commit I pointed out with the Fixes tag was introduced in 5.14-rc1 but don't think it was ever backported to earlier stable kernels. > It's the same effort direction as commit https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/usb/+/f5dfd98a80ff8d50cf4ae2820857d7f5a46cbab9 > which you added to your usb-linus branch https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/usb/+log/refs/heads/usb-linus/drivers/usb/gadget/function > Therefore I would say usb-linus. > > Please what are your rules for usb-linus and usb-next? There are some > different commits in usb-next and usb-linus changing the same file - > f_uac2.c . Likely not, but a conflict could potentially occur. I do not know > what head to rebase my further patches which also make larger changes to > that file. Since this is a fix, I'd suggest you base on Greg's usb-linus since patches queued there will be going out during the current 5.15-rc cycle. Thanks! Jack
Hi Jack, Dne 24. 09. 21 v 20:46 Jack Pham napsal(a): > Hi Pavel, > > Thanks for sending out the patch. > > On Fri, Sep 24, 2021 at 10:35:07AM +0200, Pavel Hofman wrote: >> Hi Greg, >> >> Dne 24. 09. 21 v 10:20 Greg KH napsal(a): >>> On Fri, Sep 24, 2021 at 10:00:27AM +0200, Pavel Hofman wrote: >>>> Async feedback patches broke enumeration on Windows 10 previously fixed >>>> by commit 789ea77310f0 ("usb: gadget: f_uac2: always increase endpoint >>>> max_packet_size by one audio slot"). >>>> >>>> While the existing calculation for EP OUT capture for async mode yields >>>> size+1 frame due to uac2_opts->fb_max > 0, playback side lost the +1 >>>> feature. Therefore the +1 frame addition must be re-introduced for >>>> playback. Win10 enumerates the device only when both EP IN and EP OUT >>>> max packet sizes are (at least) +1 frame. >>>> >>>> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com> >>>> Tested-by: Henrik Enquist <henrik.enquist@gmail.com> >>>> Tested-by: Jack Pham <jackp@codeaurora.org> >>>> --- >>>> drivers/usb/gadget/function/f_uac2.c | 14 ++++++++++---- >>>> 1 file changed, 10 insertions(+), 4 deletions(-) >>> >>> What commit does this fix? > > IMO it Fixes: e89bb4288378 ("usb: gadget: u_audio: add real feedback > implementation"). > >>> Should it go to stable kernel(s)? > > Yes, but probably just to 5.14 since the commit I pointed out with the > Fixes tag was introduced in 5.14-rc1 but don't think it was ever > backported to earlier stable kernels. > >> It's the same effort direction as commit https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/usb/+/f5dfd98a80ff8d50cf4ae2820857d7f5a46cbab9 >> which you added to your usb-linus branch https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/usb/+log/refs/heads/usb-linus/drivers/usb/gadget/function >> Therefore I would say usb-linus. >> >> Please what are your rules for usb-linus and usb-next? There are some >> different commits in usb-next and usb-linus changing the same file - >> f_uac2.c . Likely not, but a conflict could potentially occur. I do not know >> what head to rebase my further patches which also make larger changes to >> that file. > > Since this is a fix, I'd suggest you base on Greg's usb-linus since > patches queued there will be going out during the current 5.15-rc cycle. > Thanks for the explanation. I do not have any more small patches fixing the Win10 support now. Patches implementing EP-IN async (Playback Pitch 1000000) and the rebased Julian's multiple samplerates patch will go to Greg's usb-next and will be rebased on top of your latest SS patches. Best regards, Pavel.
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c index b9edc6787f79..f0da11409f81 100644 --- a/drivers/usb/gadget/function/f_uac2.c +++ b/drivers/usb/gadget/function/f_uac2.c @@ -665,11 +665,17 @@ 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)) { + // Win10 requires max packet size + 1 frame 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))); + // updated srate is always bigger, therefore DIV_ROUND_UP always yields +1 + max_size_bw = num_channels(chmask) * ssize * + (DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)))); + } else { + // adding 1 frame provision for Win10 + max_size_bw = num_channels(chmask) * ssize * + (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));
Async feedback patches broke enumeration on Windows 10 previously fixed by commit 789ea77310f0 ("usb: gadget: f_uac2: always increase endpoint max_packet_size by one audio slot"). While the existing calculation for EP OUT capture for async mode yields size+1 frame due to uac2_opts->fb_max > 0, playback side lost the +1 feature. Therefore the +1 frame addition must be re-introduced for playback. Win10 enumerates the device only when both EP IN and EP OUT max packet sizes are (at least) +1 frame. Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com> Tested-by: Henrik Enquist <henrik.enquist@gmail.com> Tested-by: Jack Pham <jackp@codeaurora.org> --- drivers/usb/gadget/function/f_uac2.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)