Re: uac2/hid gadget issues on Win10 hosts

[Jack Pham <jackp@codeaurora.org>]

Hi Pavel,

On Mon, Sep 06, 2021 at 02:43:25PM +0200, Pavel Hofman wrote:
> Hi Jack,
> 
> Dne 04. 09. 21 v 9:40 Jack Pham napsal(a):
> > Hi Pavel,
> > 
> > On Mon, Aug 23, 2021 at 03:17:11PM +0200, Pavel Hofman wrote:
> > > There is a problem with max packet size calculation for EP-IN. It has been
> > > discussed here recently
> > > https://www.spinics.net/lists/linux-usb/msg214615.html
> > > 
> > > The simple change in the post above fixed Win10 enumeration for me and
> > > another tester.
> > 
> > I faced the same error on Win10 and also tried the above patch and it
> > worked for me as well.  Are you planning to send a formal patch for it?
> > If so, you can add my
> > 
> > Tested-by: Jack Pham <jackp@codeaurora.org>
> > 
> > > Also, there is a problem with feedback value calculation which Win10 ignores
> > > and keeps sending the same amount of samples. The fix is to send number of
> > > samples per the actual packet, not per microframe for USB2. I have not
> > > posted the attached patch as the whole patchset will most likely be reverted
> > > for 5.15 https://www.spinics.net/lists/linux-usb/msg216042.html and I wanted
> > > to wait till the situation works out to avoid confusion. In the attached
> > > patch just change the ->c_srate_active to ->c_srate (the patch is on top of
> > > more changes for switching between multiple samplerates).
> > 
> > It doesn't look like any of the feedback EP changes got reverted for
> > 5.14 / 5.15-rc1 so it looks like the dust has settled.  Are you going to
> > send the below patch formally as well?
> > 
> 
> Thanks for testing the patch. I did not want to intrude into Jerome's plan.
> However, if Jerome is OK with the attached patch, I can submit it formally
> and continue with submitting more patches for Win10 support.
> 
> Thanks,
> 
> Pavel.

> From 26f5a49c2ddac2d5c52c4072bc756e7d15b47bc8 Mon Sep 17 00:00:00 2001
> From: Pavel Hofman <pavel.hofman@ivitera.com>
> Date: Mon, 6 Sep 2021 14:04:00 +0200
> Subject: [PATCH] usb: gadget: f_uac2: fixed EP-IN wMaxPacketSize

Thanks for this patch, it looks good to me.  Can you please send this as
a formal patch in its own mail so Greg & Felipe can properly take it?
Or if you like I can send it (if so I'd need to add my S-o-b as well).

Jack

> 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(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index ae29ff2b2b68..bdc7e6e78455 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -584,11 +584,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));
>  
> -- 
> 2.25.1
>