Re: [RFC] Sorting out dwc3 ISOC endpoints once and for all

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

Hi,

Thinh Nguyen wrote:
> Felipe Balbi wrote:
>> Hi,
>>
>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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