Re: [PATCH] uvc: gadget: uvc: Defer uvcg_complete_buffer() until .complete()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan

On 03/10/2022 16:26, Dan Vacura wrote:
Hi Dan,

Looks like I did a superset of your change here:
https://lore.kernel.org/all/20220926195307.110121-2-w36195@xxxxxxxxxxxx/


Ah-ha nice, thanks for the heads up - I'll take a better look tomorrow, but yours is definitely more comprehensive :)


I also included the uvc_video_encode_bulk() for consistency, even though
it seems to be unused code.

Out of curiosity, which setup did you test this on? I'm seeing issues on
my devices with the dwc3 controller with some of the recent performance
improvements (scatter/gather support and no_interrupt use). I've tried
to include all relevant changes in my setup, but the issues are still
present.


I'm testing with an imx8mp evaluation kit and a debix model A board (also with the imx8mp soc) as the gadget, both with a dwc3 controller. We also experience issues when it's using the scatter/gather API, to the extent that I added module parameters to set the support_sg variable false and exclude it. When using the sg code either the host or gadget would report a lot of missed isoc errors (manifesting on the host as uvcvideo reporting "USB isochronous frame lost" or on the gadget side as f_usb_uvc reporting "VS request completed with status -18") and no complete streams made it through to the host - particularly with streaming_maxpacket > 1024. Forcing the use of uvc_video_encode_isoc() instead resolved the majority of those problems.


I did also try an rpi4 (dwc2) as the gadget and didn't hit those problems with that board.


The problem this patch was solving was quite a bit different though, I hadn't considered them to be related.


Any input is appreciated,

Dan


On Mon, Oct 03, 2022 at 11:16:27AM +0100, Daniel Scally wrote:
Calling uvcg_complete_buffer() from uvc_video_encode_isoc() sometimes
causes the final isoc packet for a video frame to be delayed long
enough to cause the USB controller to drop it. The first isoc packet
of the next video frame is then received by the host, which interprets
the toggled FID bit correctly such that the stream continues without
interruption, but the first frame will be missing the last isoc
packet's worth of bytes.

To fix the issue delay the call to uvcg_complete_buffer() until the
usb_request's .complete() callback, as already happens when the data
is encoded via uvc_video_encode_isoc_sg().

Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
---
  drivers/usb/gadget/function/uvc_video.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index c00ce0e91f5d..041819a655ed 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -194,6 +194,7 @@ static void
  uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
  		struct uvc_buffer *buf)
  {
+	struct uvc_request *ureq = req->context;
  	void *mem = req->buf;
  	int len = video->req_size;
  	int ret;
@@ -213,7 +214,7 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
  		video->queue.buf_used = 0;
  		buf->state = UVC_BUF_STATE_DONE;
  		list_del(&buf->queue);
-		uvcg_complete_buffer(&video->queue, buf);
+		ureq->last_buf = buf;
  		video->fid ^= UVC_STREAM_FID;
  	}
  }
--
2.34.1




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux