On 5/5/23 15:39, Avichal Rakesh wrote:
>
>
> On 4/20/23 10:20, Laurent Pinchart wrote:
>>
>> As far as I understand, we have two ways forward here to avoid running
>> out of requests to send: sending data as quickly as possible (maximizing
>> the number of bytes sent in each packet) and filling up with 0-length
>> requests in-between, and spreading the data across packets. I'll call
>> the first one burst mode for lack of a better term.
>>
>
> Hey all,
>
> Apologies for the late reply. My not-so-long work took longer than expected.
> I tried the 2 (technically 3, but I'll go over it in a bit) approaches we had
> discussed above and the "burst" approach works pretty well. I'll attach that
> to this email, and send another email out with the other patch.
>
Here's the second test. This patch does the complete opposite of the previous one
and is based on Thinh mentioning that there was some logic built into DWC3 to
reschedule requests if the uvc driver does not queue a request for some time.
It queues usb_requests in short bursts and waits until all the queued requests
have been returned before queueing up another burst of usb_requests. This was
disastrous as it saw far more ISOC failures than before.
That likely stemmed from timing between the last request being returned and the
new burst of requests being queued, but no amount of arbitrary waiting
between last return and first queue completely eliminated the ISOC failures.
Patch, if anyone is curious:
---
drivers/usb/gadget/function/uvc.h | 1 +
drivers/usb/gadget/function/uvc_video.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 100475b1363e..e29821a80a82 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -102,6 +102,7 @@ struct uvc_video {
/* Requests */
unsigned int req_size;
+ unsigned int num_free_req;
struct uvc_request *ureq;
struct list_head req_free;
spinlock_t req_lock;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index dd1c6b2ca7c6..e3aad9c3f3ca 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -255,6 +255,7 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
struct uvc_video_queue *queue = &video->queue;
struct uvc_device *uvc = video->uvc;
unsigned long flags;
+ bool should_pump = false;
switch (req->status) {
case 0:
@@ -284,9 +285,12 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
spin_lock_irqsave(&video->req_lock, flags);
list_add_tail(&req->list, &video->req_free);
+ video->num_free_req++;
+ // Only pump more requests if we have all our usb_requests back.
+ should_pump = video->num_free_req == video->uvc_num_requests;
spin_unlock_irqrestore(&video->req_lock, flags);
- if (uvc->state == UVC_STATE_STREAMING)
+ if (uvc->state == UVC_STATE_STREAMING && should_pump)
queue_work(video->async_wq, &video->pump);
}
@@ -316,6 +320,7 @@ uvc_video_free_requests(struct uvc_video *video)
INIT_LIST_HEAD(&video->req_free);
video->req_size = 0;
+ video->num_free_req = 0;
return 0;
}
@@ -332,10 +337,12 @@ uvc_video_alloc_requests(struct uvc_video *video)
* max_t(unsigned int, video->ep->maxburst, 1)
* (video->ep->mult);
+ video->num_free_req = 0;
video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);
if (video->ureq == NULL)
return -ENOMEM;
+ video->num_free_req = 0;
for (i = 0; i < video->uvc_num_requests; ++i) {
video->ureq[i].req_buffer = kmalloc(req_size, GFP_KERNEL);
if (video->ureq[i].req_buffer == NULL)
@@ -357,6 +364,7 @@ uvc_video_alloc_requests(struct uvc_video *video)
sg_alloc_table(&video->ureq[i].sgt,
DIV_ROUND_UP(req_size - UVCG_REQUEST_HEADER_LEN,
PAGE_SIZE) + 2, GFP_KERNEL);
+ video->num_free_req++;
}
video->req_size = req_size;
@@ -400,6 +408,7 @@ static void uvcg_video_pump(struct work_struct *work)
req = list_first_entry(&video->req_free, struct usb_request,
list);
list_del(&req->list);
+ video->num_free_req--;
spin_unlock_irqrestore(&video->req_lock, flags);
/*
@@ -450,6 +459,7 @@ static void uvcg_video_pump(struct work_struct *work)
spin_lock_irqsave(&video->req_lock, flags);
list_add_tail(&req->list, &video->req_free);
+ video->num_free_req++;
spin_unlock_irqrestore(&video->req_lock, flags);
return;
}
@@ -527,4 +537,3 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)
V4L2_BUF_TYPE_VIDEO_OUTPUT, &video->mutex);
return 0;
}
-
--
