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

[Dan Vacura <w36195@motorola.com>]

Hi Dan,

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

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.

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@ideasonboard.com>
> ---
>  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
>