| Message ID | 20180810190545.GA4119@uda0271908 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | g_audio regression on dwc3 udc | expand |
Hi Bin, On 8/10/2018 12:05 PM, Bin Liu wrote: > Hi Felipe, > > I noticed a regression on g_audio (uac2) with dwc3 udc on v4.14: > > - nosound in playback > - playback can only play once, the following error happens from the 2nd > playback > > aplay: set_params:1361: Unable to install hw params: > > A manual bisect tells the two issues are seperate and the regression > happens in v4.11-rc2 with the following two patches. > > 40d829fb2ec63 ("usb: dwc3: gadget: Correct ISOC DATA PIDs for short packets") > > which cases the first no-sound issue. > > ep->mult is already 1 from g_audio, so mult-- leads to incorrect value. > The following change solves it. I believe v4.14.40 is missing this patch regarding that issue: ec5bb87e4e2a ("usb: dwc3: gadget: Fix PCM1 for ISOC EP with ep->mult less than 3") > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index f92e2f2c7fbe..758cc258895c 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -930,10 +930,10 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, > unsigned int mult = ep->mult - 1; > unsigned int maxp = usb_endpoint_maxp(ep->desc); > > - if (length <= (2 * maxp)) > + if (mult && length <= (2 * maxp)) > mult--; > > - if (length <= maxp) > + if (mult && length <= maxp) > mult--; > > trb->size |= DWC3_TRB_SIZE_PCM1(mult); > > cf3113d893d4 ("usb: dwc3: gadget: properly increment dequeue pointer on ep_dequeue") > > which causes the multiple playback failure. The following hack covers it. > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index f92e2f2c7fbe..758cc258895c 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1411,7 +1411,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > if (r == req) { > /* wait until it is processed */ > dwc3_stop_active_transfer(dwc, dep->number, true); > - > +#if 0 > /* > * If request was already started, this means we had to > * stop the transfer. With that we also need to ignore > @@ -1473,6 +1473,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > dwc3_ep_inc_deq(dep); > } > } > +#endif > goto out1; > } > dev_err(dwc->dev, "request %pK was not queued to %s\n", > > With the two changes above, g_audio works fine with v4.14.40, but not with > v4.18-rc7, in which the console is flooded with following message. > > u_audio_iso_complete: iso_complete status(-18) 0/256 > Pierre Le Magourou attempted to fix the second issue with this patch (though the patch may need rework): https://patchwork.kernel.org/patch/10509639/ You can try it out. It works fine when I tested it on HAPS platform (using Felipe's next branch). BR, Thinh
Hi Thinh, On Fri, Aug 10, 2018 at 09:36:30PM +0000, Thinh Nguyen wrote: > Hi Bin, > > On 8/10/2018 12:05 PM, Bin Liu wrote: > > Hi Felipe, > > > > I noticed a regression on g_audio (uac2) with dwc3 udc on v4.14: > > > > - nosound in playback > > - playback can only play once, the following error happens from the 2nd > > playback > > > > aplay: set_params:1361: Unable to install hw params: > > > > A manual bisect tells the two issues are seperate and the regression > > happens in v4.11-rc2 with the following two patches. > > > > 40d829fb2ec63 ("usb: dwc3: gadget: Correct ISOC DATA PIDs for short packets") > > > > which cases the first no-sound issue. > > > > ep->mult is already 1 from g_audio, so mult-- leads to incorrect value. > > The following change solves it. > > I believe v4.14.40 is missing this patch regarding that issue: > ec5bb87e4e2a ("usb: dwc3: gadget: Fix PCM1 for ISOC EP with ep->mult > less than 3") > > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index f92e2f2c7fbe..758cc258895c 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -930,10 +930,10 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, > > unsigned int mult = ep->mult - 1; > > unsigned int maxp = usb_endpoint_maxp(ep->desc); > > > > - if (length <= (2 * maxp)) > > + if (mult && length <= (2 * maxp)) > > mult--; > > > > - if (length <= maxp) > > + if (mult && length <= maxp) > > mult--; > > > > trb->size |= DWC3_TRB_SIZE_PCM1(mult); > > > > cf3113d893d4 ("usb: dwc3: gadget: properly increment dequeue pointer on ep_dequeue") > > > > which causes the multiple playback failure. The following hack covers it. > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index f92e2f2c7fbe..758cc258895c 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -1411,7 +1411,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > > if (r == req) { > > /* wait until it is processed */ > > dwc3_stop_active_transfer(dwc, dep->number, true); > > - > > +#if 0 > > /* > > * If request was already started, this means we had to > > * stop the transfer. With that we also need to ignore > > @@ -1473,6 +1473,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > > dwc3_ep_inc_deq(dep); > > } > > } > > +#endif > > goto out1; > > } > > dev_err(dwc->dev, "request %pK was not queued to %s\n", > > > > With the two changes above, g_audio works fine with v4.14.40, but not with > > v4.18-rc7, in which the console is flooded with following message. > > > > u_audio_iso_complete: iso_complete status(-18) 0/256 > > > > Pierre Le Magourou attempted to fix the second issue with this patch > (though the patch may need rework): > https://patchwork.kernel.org/patch/10509639/ > > You can try it out. It works fine when I tested it on HAPS platform > (using Felipe's next branch). Thanks for the hints. With the two patches you pointed, g_audio works fine on v4.14.40. Regards, -Bin.
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index f92e2f2c7fbe..758cc258895c 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -930,10 +930,10 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, unsigned int mult = ep->mult - 1; unsigned int maxp = usb_endpoint_maxp(ep->desc); - if (length <= (2 * maxp)) + if (mult && length <= (2 * maxp)) mult--; - if (length <= maxp) + if (mult && length <= maxp) mult--; trb->size |= DWC3_TRB_SIZE_PCM1(mult); cf3113d893d4 ("usb: dwc3: gadget: properly increment dequeue pointer on ep_dequeue") which causes the multiple playback failure. The following hack covers it. diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index f92e2f2c7fbe..758cc258895c 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1411,7 +1411,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, if (r == req) { /* wait until it is processed */ dwc3_stop_active_transfer(dwc, dep->number, true); - +#if 0 /* * If request was already started, this means we had to * stop the transfer. With that we also need to ignore @@ -1473,6 +1473,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, dwc3_ep_inc_deq(dep); } } +#endif goto out1; } dev_err(dwc->dev, "request %pK was not queued to %s\n",
Hi Felipe, I noticed a regression on g_audio (uac2) with dwc3 udc on v4.14: - nosound in playback - playback can only play once, the following error happens from the 2nd playback aplay: set_params:1361: Unable to install hw params: A manual bisect tells the two issues are seperate and the regression happens in v4.11-rc2 with the following two patches. 40d829fb2ec63 ("usb: dwc3: gadget: Correct ISOC DATA PIDs for short packets") which cases the first no-sound issue. ep->mult is already 1 from g_audio, so mult-- leads to incorrect value. The following change solves it. With the two changes above, g_audio works fine with v4.14.40, but not with v4.18-rc7, in which the console is flooded with following message. u_audio_iso_complete: iso_complete status(-18) 0/256 Regards, -Bin.