Hi, Thinh Nguyen wrote: > Felipe Balbi wrote: >> Hi, >> >> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: >>>>>>> Would there be any obvious draw-back to going down this route? The thing >>>>>>> is that, as it is, it seems like we will *always* have some corner case >>>>>>> where we can't guarantee that we can even start a transfer since there's >>>>>>> no upper-bound between XferNotReady and gadget driver finally queueing a >>>>>>> request. Also, I can't simply read DSTS for the frame number because of >>>>>>> top-most 2 bits. >>>>>>> >>>>>> For non-affected version of the IP, the xfernotready -> starttransfer >>>>>> time will have to be off by more than a couple seconds for the driver >>>>>> to produce an incorrect 16-bit frame number. If you're seeing errors >>>>>> here, maybe we just need to code review the relevant sections to make >>>>>> sure the 14/16-bit and rollover conditions are all handled correctly. >>>>> I think what Felipe may see is some delay in the system that causes the >>>>> SW to not handle XferNotReady event in time. We already have the "retry" >>>>> method handle that to a certain extend. >>>>> >>>>>> But I can't think of any obvious drawbacks of the quirk, other than >>>>>> doing some unnecessary work, which shouldn't produce any bad >>>>>> side-effects. But we haven't really tested that. >>>>>> >>>>> The workaround for the isoc_quirk requires 2 tries sending >>>>> START_TRANSFER command. This means that you have to account the delay of >>>>> that command completion plus potentially 1 more END_TRANSFER completion. >>>>> That's why the quirk gives a buffer of at least 4 uframes of the >>>>> scheduled isoc frame. So, it cannot schedule immediately on the next >>>>> uframe, that's one of the drawbacks. >>>>> >>>>> >>>>> Hi Felipe, >>>>> >>>>> Since you're asking this, it means you're still seeing issue with your >>>>> setup despite retrying to send START_TRANSFER command 5 times. What's >>>>> the worse delay responding to XferNotReady you're seeing in your setup? >>>> There's no upper-bound on how long the gadget will take to enqueue a >>>> request. We see problems with UVC gadget all the time. It can take a lot >>>> of time to decide to enqueue data. >>> That's why there's a mechanism in the controller to return bus-expiry >>> status to let the SW know if it had scheduled isoc too late. SW can do 2 >>> things: 1) re-schedule at a later timer or 2) send END_TRANSFER command >>> to wait for the next XferNotReady to try again. >> All of this is still rather flakey. Can I send consecutive END_TRANSFER >> commands and get new XferNotReady at any moment? Consider this >> situation: >> >> . transfer started >> . transfer completes with status Missed ISOC >> . driver issues END_TRANSFER (as required by docs) >> . XferNotReady fires >> . driver updates current frame number >> . several mS of nothing >> . finally gadget enqueues a transfer >> . Start Transfer command >> . completes with Bus Expiry >> >> Can I issue *another* END_TRANSFER at this point? I don't even have a >> valid transfer resource since transfer wasn't started. > Yes you can. If the START_TRANSFER command completes, you will get the > transfer resource index to use for END_TRANSFER command (regardless of > bus-expiry status). > > However, there's a chance if the SW somehow keeps handling the > XferNotReady event late over and over and the isoc transfer never get > started. That's where you can create a quirk and use frame number from > DSTS instead. > I thought about this again. I think the best way to handle this is the followings: * For service interval less than 2 seconds, use DSTS and XferNotReady frame number to calculate and schedule isoc. * For service interval of 2 second or larger, just use XferNotReady frame number. * If START_TRANSFER command still fails with bus-expiry, then send END_TRANSFER command to restart on a new XferNotReady. This should only happen if the SW is too slow to handle XferNotReady event for over 2 seconds. This case should not happen often. Note: bus-expiry happens when |current frame - scheduled frame| > 4 seconds. This guarantees that the DWC3 will retry to schedule the isoc transfer again. There's no need to create a quirk for the controller. What do you think? BR, Thinh
