On Wed, Apr 19, 2023 at 3:38 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote: > > On Wed, Apr 19, 2023, Avichal Rakesh wrote: > > On Wed, Apr 19, 2023 at 2:49 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote: > > > > > > On Tue, Apr 18, 2023, Avichal Rakesh wrote: > > > > On Tue, Apr 18, 2023 at 6:07 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > > > On Tue, Apr 18, 2023 at 03:45:53PM -0700, Avichal Rakesh wrote: > > > > > > I see, and I think I understand Greg's previous comment better as > > > > > > well: The UVC driver isn't falling behind on the video stream, it is > > > > > > falling behind the usb controller's monotonic isoc stream. > > > > > > > > > > > > From what I can see, this leaves us in an interesting place: UVC > > > > > > allows the host to configure the camera's output resolution and fps, > > > > > > which effectively controls how fast the camera is generating data. > > > > > > This is at odds with the UVC gadget driver, which currently packs each > > > > > > video frame into as few usb_requests as possible (using the full > > > > > > available size in usb_requests). Effectively, the UVC gadget driver > > > > > > attempts to use the "full" bandwidth of isoc transfers even when the > > > > > > camera isn't generating data fast enough. For example, in my > > > > > > observations: 1 video frame is ~22kB. At 30fps, this represents 1/30 > > > > > > of the amount of data the camera would generate in a second. This 22kB > > > > > > is split into 8 usb_requests which is about 1/1000 the number of > > > > > > requests UVC driver needs to generate per second to prevent isoc > > > > > > failures (assuming 125us monotonic uframes). Assuming some fudge > > > > > > factor from the simplifications in your explanation gives the uvc > > > > > > driver some extra leeway with request queuing, we're still roughly two > > > > > > order of magnitudes out of sync. Even with perfect 'complete' > > > > > > callbacks and video frame encodings, an underrun seems inevitable. > > > > > > Data is being generated at a far slower rate than it is being > > > > > > transferred. Does this reasoning seem valid? > > > > > > > > > > > > Just as a test I'll try updating the UVC driver to consume 266 > > > > > > usb_requests per video frame (~1/30 of 8000), which should be enough > > > > > > to keep the usb controller queue occupied for ~1/30s. Ideally, by the > > > > > > time the controller queue is empty, the camera would have produced a > > > > > > new frame. This doesn't solve the issue with latencies around callback > > > > > > and an isoc failure might still happen, hopefully the failure > > > > > > frequency is reduced because UVC queues enough requests per video > > > > > > frame to not starve the controller's queue while waiting on a new > > > > > > frame and the only way they go out of sync is from 'complete' callback > > > > > > timings. I am assuming this has been tried before, but my LKML search > > > > > > skills are failing and I can't find much on it. > > > > > > > > > > Note that there's nothing wrong with submitting a 0-length isochronous > > > > > transfer. If there's no data left but you still need to send > > > > > _something_ in order to fill out the remaining slots in the controller's > > > > > schedule, this is a good way to do it. > > > > > > > > > Oh, this is very good to know, thank you!! We just need to reach a > > > > steady state of UVC queuing enough requests monotonically (even if > > > > they are empty), and the usb controller calling the 'complete' > > > > callback to give it more requests to queue. Although I wonder how the > > > > host's UVC driver would interpret the zero length packets, if it would > > > > even care. > > > > > > By the usb spec, for IN direction, if there's no data available and the > > > host requests for data, then the device will send a zero-length data > > > packet. This is what the dwc3 controller will do. So regardless whether > > > you prepare and queue a 0-length request or not, the host will receive > > > it. > > > > > If this is the case: the usb controller sends a 0 length packet to the > > host when uvc gadget driver doesn't queue anything, what stops the > > controller from assuming that the usb_request queued by a gadget > > driver is always for the next available uframe, and not for one in the > > past? This is effectively the "always reschedule" suggestion you made > > before but as a default instead of specific to uvc. Are there cases > > where we would want to tell the gadget driver that it fell behind? > > Apologies if I am missing something fundamental here, it feels like I > > am :(. > > No, the controller doesn't assume that. It won't queue for the future > uframe if it's stale. The UVC gadget driver will need to keep feeding > requests until it catches up to the current uframe. Remember that the > isoc data is time sensitive, if it's not gone out at a specific uframe, > then it's dropped. > > The controller driver can end the current isoc stream and reschedule new > requests for the future uframe so that they won't be considered "stale". > Oh I see, the hardware/controller doesn't make that assumption, and to mimic that behavior the driver will have to detect empty queues and reschedule requests which runs into its own timing issues with the callbacks. I think I understand! - Avi.
