On Mon, Apr 17, 2023, Avichal Rakesh wrote: > > On 4/17/23 06:49, Greg KH wrote: > > On Fri, Apr 14, 2023 at 02:03:02PM -0700, Avichal Rakesh wrote: > > > >> This problem may be further exaggerated by the DWC3 controller driver > >> (which is what my device has) not setting the IMI flag when > >> no_interrupt flag is set > >> (https://urldefense.com/v3/__https://lore.kernel.org/all/ced336c84434571340c07994e3667a0ee284fefe.1666735451.git.Thinh.Nguyen@xxxxxxxxxxxx/)?__;!!A4F2R9G_pg!fB0krUX6qsOXrBsOHzLstVFWqTLL9ncwGFXxlxK0eedLY_29XdwjXRtbO-EWv1eX5okN1rOeBZspZ21KSb5b$ > >> UVC Gadget Driver sets the no_interrupt flag for ~3/4 of its queued > >> usb_request, so an ISOC failure may not immediately interrupt the UVC > >> gadget driver, leaving more time for the frame to finish encoding. > >> > >> I couldn't find any concrete error handling rules in the UVC specs, so > >> I am not sure what the proper solution here is. To try out, I created > >> a patch (attached below) that dequeues all queued usb_requests from > >> the endpoint in case of an ISOC failure and clears the uvc buffer > >> queue. This eliminated the partial frames with no perceivable frame > >> drops. > >> > >> So my questions here are: > >> 1. Is this a known issue, and if so are there workarounds for it? > >> 2. If the answer to above is "No", does the explanation and mitigation > >> seem reasonable? > >> > >> Patch follows (mostly for illustration, I can formalize it if > >> needed!). It adds a new 'req_inflight' list to track queued > >> usb_requests that have not been given back to the gadget driver and > >> drops all the queued requests in case of an ISOC failure. The other > >> changes are for the extra bookkeeping required to handle dropping all > >> frames. I haven't been able to confirm it, but as far as I can tell > >> the issue exists at ToT as well. > >> Perhaps this conversation with Jeff may explain the issues you observed: https://lore.kernel.org/all/20221021164349.fft4yqnxuztsqdeu@xxxxxxxxxxxx/ To summarized the long conversation, the UVC needs to maintain a constant queue of usb requests. Each request is scheduled for a specific interval. If the UVC couldn't keep up feeding requests to the controller, then we will have stale requests and missed isoc. >From the discussion and review, the UVC couldn't queue requests fast enough. The problem is exacerbated when you reduce the interrupt frequency with no_interrupt setting. The dwc3 driver has some basic mechanism to detect for underrun by checking if the queue is empty, but that's not enough to workaround it. Your suggestion to dequeue the request would mean the controller driver will reschedule the isoc transfer again, which reschedule the next request for a new interval (potentially avoid being stale). That's fine, but that may not be a great solution because we're still playing catch up and the missed isoc already happened. You may try to do the followings to mitigate this issue: 1) Make sure UVC to only queuing rate is constant. Perhaps queue dummy requests in between time gaps if need be? 2) Enhance dwc3 to detect underrun from UVC on request queue. ie. If the queue is empty and a new request is queue, reschedule the isoc transfer. This will break some other devices if time guarantee is required. So perhaps an additional flag is needed for this change of behavior. BR, Thinh
