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. > The best "workaround" I can think of would be to delay END_TRASFER until > ep_queue time. > >>> Usually I hear this from folks using UVC gadget with a real sensor on >>> the background. >>> >>> I've seen gadget enqueueing as far as 20 intervals in the future. But >>> remember, there's no upper-bound. And that's the problem. If we could >>> just read the frame number from DSTS and use that, we wouldn't have any >>> issues. But since DSTS only contains 14 our of the 16 bits the >>> controller needs, then we can't really use that. >> You can create another quirk for devices that have this behavior to use >> frame number in DSTS instead. As John had pointed out and mentioned, >> this will only work if the service interval and the delay in the >> scheduling of isoc is within 2 seconds. > well, that's better than nothing, but there's no upper-bound for the > gadget driver, really. > >> You will need to calculate this value along with BIT(15) and BIT(14) of >> XferNotReady for rollovers. >> >>> To me, it seems like this part of the controller wasn't well >>> thought-out. These extra two bits, perhaps, should be internal to the >>> controller and SW should have no knowledge that they exist. >> These values are internal. SW should not have knowledge of it. This >> implementation will not follow the programming guide and should be used >> as a quirk for devices that are too slow to handle the XferNotReady >> event but want to schedule isoc immediately after handling the event. > They are *not* internal if SW needs to know that to start a transfer > properly it needs these extra two bits :-) What I meant to say was that > we should never have a 16-bit frame number. Only 14 bits. But in any > case, we can't change the HW now :-) > Yeah... I made a correction in my previous email that it's not internal. But like I said, we can create a quirk to use DSTS frame number to workaround this issue with some limitations. BR, Thinh
