On Fri, Nov 15, 2019 at 04:06:10PM -0500, Alan Stern wrote: > On Thu, 14 Nov 2019, Thinh Nguyen wrote: > > > Michael Olbrich wrote: > > > >>> How about changing the gadget driver instead? For frames where the UVC > > >>> gadget knows no video frame data is available (numbers 4, 8, 12, and so > > >>> on in the example above), queue a zero-length request. Then there > > >>> won't be any gaps in the isochronous packet stream. > > >> What Alan suggests may work. Have you tried this? > > > Yes and it works in general. There are however some problems with that > > > approach that I want to avoid: > > > > > > 1. It adds extra overhead to handle the extra zero-length request. > > > Especially for encoded video the available bandwidth can be quite a bit > > > larger that what is actually used. I want to avoid that. > > This comment doesn't seem to make sense. If the available bandwidth is > much _larger_ than what is actually used, what's the problem? You > don't run into difficulties until the available bandwidth is too > _small_. > > The extra overhead of a zero-length request should be pretty small. > After all, the gadget expects to send a packet for every frame anyway, > more or less. My current test-case is video frames with 450kB on average at 30fps. This currently results in ~10 CPU load for the threaded interrupt handler. At least in my test, filling the actual video data into the frame has very little impact. So if I reserve 900kB to support occasionally larger video frames, then I expect that this CPU load will almost double in all cases, not just when the video frames are larger. > > > 2. The UVC gadget currently does no know how many zero-length request must > > > added. So it needs fill all available request until a new video frame > > > arrives. With the current 4 requests that is not a problem right now. But > > > that does not scale for USB3 bandwidths. So one thing that I want to do is > > > to queue many requests but only enable the interrupt for a few of than. > > > From what I can tell from the code, the gadget framework and the dwc3 > > > driver should already support this. > > > This will result in extra latency. There is probably an acceptable > > > trade-off with an acceptable interrupt load and latency. But I would like > > > to avoid that if possible. > > There are two different situations to consider: > > In the middle of a video stream, latency isn't an issue. > The gadget should expect to send a new packet for each frame, > and it doesn't know what to put in that packet until it > receives the video data or it knows there won't be any data. > > At the start of a video stream, latency can be an issue. But > in this situation the gadget doesn't have to send 0-length > requests until there actually is some data available. > > Either way, it should be okay. > > As far as interrupt load is concerned, I don't see how it relates to > the issue of sending 0-length requests. Maybe I don't understand, how 0-length requests work. My current understanding is, that they are queued like any other request. If I want to reduce the number of interrupts then I need to queue more requests and only ask for an interrupt for some of them. This means that potentially a lot of 0-length requests requests are queued when a new video frame arrives and this means extra latency for the frame. I think the worst-case latency is 2x the time between two interrupts. So less interrupts mean more latency. The stop/start transfer this patch implements, the video frame can be sent immediately without any extra latency. > > I think I understand the problem you're trying to solve now. > > > > The dwc3 driver does not know that there's a gap until after a new > > request was queued, which then it will send an END_TRANSFER command and > > dequeue all the requests to restart the transfer due to missed_isoc. > > We do this because the dwc3 driver does not know whether the new request > > is actually stale data, and we should not change this behavior. > > > > Now, with UVC, it needs to communicate to the dwc3 driver that there > > will be a gap after a certain request (and that the device is expecting > > to send 0-length data). This is not a normal operation for isoc > > transfer. You may need to introduce a new way for the function driver to > > do that, possibly a new field in usb_request structure to indicate that. > > However, this seems a little awkward. Maybe others can comment on this. I'm not sure how this is supposed to work. What exactly can the dwc3 driver / hardware do to handle a gap? > Note that on the host side, there is a difference between receiving > a 0-length packet and receiving no packet at all. As long as both the > host and the gadget expect the isochronous stream to be running, there > shouldn't be any gaps if you can avoid it. Huh, so how is this handled on other hardware? From what I can tell the UVC gadget works with other drivers and I've not found any special handling for this. Is there no packet sent or are 0-length packet generated implicitly somewhere? Regards, Michael -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
