Hi Felipe,
On 11/7/2018 10:58 PM, Felipe Balbi wrote:
> 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_number was updated and make sure we start the request
> to the next valid interval.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/dwc3/core.h | 5 +++++
> drivers/usb/dwc3/gadget.c | 11 +++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 131028501752..306a2dd75ed5 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -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];
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index d8c7ad0c22e8..00fe01a01977 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -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);
> }
>
> @@ -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_timestamp may not correspond to
the frame_number from 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.
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))
Thinh
