usb: dwc3: gadget: check if dep->frame_number is still valid
PATCH 4/4 usb: dwc3: gadget: check if dep->frame_number is still valid
Hi,
Quoted 81 lines · Thinh Nguyen
Quoted 66 lines · Felipe Balbi
Gadget driver may take an unbounded amount of time to queue requests after XferNotReady. This is important for isochronous endpoints which need to be started for a specific (micro-)frame.
Before kicking the transfer, let's check how much time has elapsed since
dep->frame_numberwas updated and make sure we start the request to the next valid interval.Signed-off-by Felipe Balbi <[email protected]>--- drivers/usb/dwc3/core.h | 5 +++++ drivers/usb/dwc3/gadget.c | 11 +++++++++++ 2 files changed, 16 insertions(+)drivers/usb/dwc3/core.h +5
-0@@ -651,6 +651,7 @@ struct dwc3_event_buffer {* @number: endpoint number (1 - 15) * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK * @resource_index: Resource transfer index +* @frame_timestamp: timestamp of most recent frame number * @frame_number: set to the frame number we want this transfer to start (ISOC) * @interval: the interval on which the ISOC transfer is started * @name: a human readable name e.g. ep1out-bulk @@ -697,7 +698,11 @@ struct dwc3_ep {u8 number;u8 type;u8 resource_index; + +u64 frame_timestamp;u32 frame_number; +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff +u32 interval;char name[20 ];drivers/usb/dwc3/gadget.c +10
-0@@ -1268,12 +1268,22 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)static int __dwc3_gadget_start_isoc (struct dwc3_ep *dep) { +u64 current_timestamp; +u64 diff_timestamp; +u32 elapsed_frames; +if (list_empty (&dep->pending_list)) { dep->flags |=DWC3_EP_PENDING_REQUEST ;return -EAGAIN ; } + current_timestamp =ktime_get_ns (); + diff_timestamp = current_timestamp - dep->frame_timestamp; + elapsed_frames =DIV_ROUND_UP_ULL (diff_timestamp,125000 ); + + dep->frame_number += elapsed_frames; dep->frame_number =DWC3_ALIGN_FRAME (dep); +return __dwc3_gadget_kick_transfer (dep); }drivers/usb/dwc3/gadget.c static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, · line 2336
Quoted 7 lines
drivers/usb/dwc3/gadget.c +1
-0@@ -2320,6 +2330,7 @@ static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep, const struct dwc3_event_depevt *event) { dep->frame_number = event->parameters; + dep->frame_timestamp =ktime_get_ns (); } static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,This may not be enough. The
dep->frame_timestampmay not correspond to theframe_numberfrom XferNotReady event. When there's system latency (which is possible when this failure happens), the time the driver handle the event may be a few uframes passed the time the controller's XferInProgress uframe parameter.
Quoted 3 lines
Rather than starting the isoc transfer immediately on the next interval. How about starting the transfer with some minimum buffer uframes just like before? (e.g.
frame_number+max(4, interval))
The problem with this is cases with interval of 1ms. This will result in a 4ms delay. I really want to start transfer as soon as possible and the timestamp trick seems to be the best idea so far, without resorting to 4 intervals delay. We do, however, have the possibility that this will start 2 intervals in the future because of the usage of DIV_ROUND_UP_ULL() and because of how DWC3_ALIGN_FRAME() is implemented.
I understand what you're saying, though, but it seems like we don't have to avoid that case completely. We can only make it less likely.