Hi,
Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>> On 4/9/2018 4:28 AM, Felipe Balbi wrote:
>>>> In case we get an event with status set to Missed Isoc, this means we
>>>> have missed an isochronous interval and should issue End Transfer
>>>> command and wait for the following XferNotReady.
>>>
>>> Why does DWC3 need to issue End Transfer if there are still queued requests?
>>
>> Without XferNotReady, we won't have a reliable way to know the uFrame
>> number. Read the Isochronous programming sequence from your databook.
>
> Right. We need XferNotReady to know when to start isoc transfer. But if
> there are still queued requests, DWC3 can just wait to see if any of
> them will succeed to continue with the transfer just as how DWC3 is
> handling it now.
That's not what the databook says though. And that's also not intention
of how the code is written as of now either. The way the code is written
is the following:
queue() -> XferNotReady -> start_isoc() -> if (missed) do_nothing() ->
queue() -> end_transfer.
That's not really waiting for the queue to be consumed, it's just
delaying end transfer until we get another queue(). IOW, it just
*happens* to give the controller time to go through the list of started
requests.
> If we end and restart the transfer right away, then we may lose more
> isoc data than necessary (due to isoc scheduling at least 4 uFrame
> ahead of time). Is there something you see that doesn't work with the
> current implementation?
Not _really_, I'm just trying to make the code easier to read and, I
think, I've achieved that. Now, if we need to delay end transfer in the
case where we have more requests in the controller's queue, that's easy
enough to implement:
@@ -2371,7 +2371,8 @@ static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
if (event->status & DEPEVT_STATUS_BUSERR)
status = -ECONNRESET;
- if (event->status & DEPEVT_STATUS_MISSED_ISOC) {
+ if (event->status & DEPEVT_STATUS_MISSED_ISOC &&
+ list_empty(&dep->started_list) {
status = -EXDEV;
stop = true;
}
I'm not sure this is a good idea though. Once we miss an interval, don't
we need to know the next frame when transfer needs to be scheduled?
Meaning we would need XferNotReady to properly schedule the new
transfer?
--
balbi
Attachment:
signature.asc
Description: PGP signature
