On Tue, Oct 25, 2022 at 01:34:01AM -0500, Jeff Vanhoof wrote: > Hi, > > During the queuing up of requests from the UVC Gadget Driver to DWC3 for one > frame, if a missed isoc event occurs then it is possible for the next > consecutive frame(s) to also see missed isoc related errors as a result, > presenting to the user as a large video stall. > > This issue appears to have come in with the skip interrupt implementation in > the UVC Gadget Driver: > > usb: gadget: uvc: decrease the interrupt load to a quarter > https://lore.kernel.org/r/20210628155311.16762-6-m.grzeschik@xxxxxxxxxxxxxx > > Below is an example flow of how the issue can occur (and why). > > For example (ISOC use case): > 1) DWC3 driver has 4 requests queued up from the UVC Gadget Driver. > > 2) First request has IOC bit set due to no_interrupt=0 also being set, and IMI > bit is set to detect missed ISOC. > > 3) Requests 2,3,4 do not have IOC bit set due to no_interrupt=1 being set for > them. (Note: Whether or not the IMI bit is set for these requests does not > matter, issue can still crop up as there is no guarantee that request 2,3,4 > will see a missed isoc event) > > 4) First request gets a missed isoc event and DWC3 returns the req and error to > UVC Gadget Driver. > > 5) UVC Gadget Driver, in uvc_video_complete, proceeds to cancel the queue by > calling uvcg_queue_cancel. > > 6) UVC Gadget Driver stops sending additional requests for the current frame. > > 7) DWC3 will still have requests 2,3,4 queued up and sitting in its > started_list as these requests are not given back to the UVC gadget driver > because they each have no_interrupt=1 set, and the DWC3 driver will not have > any additional interrupts triggered for them as a result. > > 8) Approximately 30-100ms later a new frame enters the UVC Gadget Driver (from > V4L2), and it proceeds to send additional requests to the DWC3 driver. > > 9) Because requests 2,3,4 are still sitting in the started_list of the dwc3 > driver, the driver does not stop and restart the transmission that normally > helps it recover from the missed isoc situation (this usually happens in > between frames). > > 10) Some of the requests from the new frame will have no_interrupt=0 set, but > these requests will be considered missed/late by the DWC3 controller. > > 11) Because these new requests have the IOC bit set (and possibly IMI), > interrupts will be triggered causing the DWC3 Driver to return the req and > error to the UVC Gadget Driver. > > 12) And if the last set of requests sent by the UVC Gadget Driver have > "no_interrupt=1" set, then DWC3 may not interrupt further until new requests > come in, and the cycle of frame drops/errors will continue. > > I have briefly mentioned this issue in another conversation with Thinh. At the > time he mentioned that 3 things could possibly be done to help resolve this > issue: > > 1) The UVC Gadget Driver should ensure that the last requests queued to DWC3 > must always have "no_interrupt=0" set. > > 2) DWC3 can detect stale requests, stop the transmission and give back the > requests to the UVC Gadget Driver, and restart the transmission for the new set > of requests. > > 3) Set "no_interrupt=0" for each request. > > I have tested out various implementations for all 3 possibilities and they each > seem to work ok. Note that these test implementations are not ready for prime > time, but served as a way to prove that potential changes in these areas could > help to resolve this issue. > > I believe that a change for the UVC Gadget Driver should be made, but it also > makes sense for the DWC3 driver to also attempt to recover from this situation > if possible. > > Does anyone have an opinion on the best way to proceed? Please see this set of patches and the discussion around them: https://lore.kernel.org/r/20221018215044.765044-1-w36195@xxxxxxxxxxxx Some of them are already queued up in my tree and in linux-next, can you try that? There are others for the dwc3 driver on the mailing list as well, testing those would be wonderful if you could do that. thanks, greg k-h
