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

[Felipe Balbi <felipe.balbi@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2934 bytes --]


Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>>>
>>>  static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct
>>> dwc3_request *req)
>>>
>>>
>>> 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.

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.

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.

In any case, this is the biggest sort of issues in DWC3 right now :-)

Anything else seems to behave nicely without any problems.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]