Re: [PATCH v2 1/3] usb: gadget: uvc: calculate the number of request depending on framesize

[Dan Vacura <w36195@motorola.com>]

Hi Michael,

On Mon, May 30, 2022 at 12:38:46AM +0200, Michael Grzeschik wrote:
> The current limitation of possible number of requests being handled is
> dependent on the gadget speed. It makes more sense to depend on the
> typical frame size when calculating the number of requests. This patch
> is changing this and is using the previous limits as boundaries for
> reasonable minimum and maximum number of requests.
> 
> For a 1080p jpeg encoded video stream with a maximum imagesize of
> e.g. 800kB with a maxburst of 8 and an multiplier of 1 the resulting
> number of requests is calculated to 49.
> 
>         800768         1
> nreqs = ------ * -------------- ~= 49
>           2      (1024 * 8 * 1)
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Thanks for the patch. This improves the usb2 performance on our
android 5.10 kernel with dwc3 controller. Any reason to not cc stable
lines? This does help older releases as well.

Tested-by: Dan Vacura <w36195@motorola.com>

> 
> ---
> v1 -> v2: - using clamp instead of min/max
>           - added calculation example to description
> 	  - commented the additional division by two in the code
> 
>  drivers/usb/gadget/function/uvc_queue.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
> index d25edc3d2174e1..eb9bd9d32cd056 100644
> --- a/drivers/usb/gadget/function/uvc_queue.c
> +++ b/drivers/usb/gadget/function/uvc_queue.c
> @@ -44,7 +44,8 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>  {
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>  	struct uvc_video *video = container_of(queue, struct uvc_video, queue);
> -	struct usb_composite_dev *cdev = video->uvc->func.config->cdev;
> +	unsigned int req_size;
> +	unsigned int nreq;
>  
>  	if (*nbuffers > UVC_MAX_VIDEO_BUFFERS)
>  		*nbuffers = UVC_MAX_VIDEO_BUFFERS;
> @@ -53,10 +54,16 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>  
>  	sizes[0] = video->imagesize;
>  
> -	if (cdev->gadget->speed < USB_SPEED_SUPER)
> -		video->uvc_num_requests = 4;
> -	else
> -		video->uvc_num_requests = 64;
> +	req_size = video->ep->maxpacket
> +		 * max_t(unsigned int, video->ep->maxburst, 1)
> +		 * (video->ep->mult);
> +
> +	/* We divide by two, to increase the chance to run
> +	 * into fewer requests for smaller framesizes.
> +	 */
> +	nreq = DIV_ROUND_UP(DIV_ROUND_UP(sizes[0], 2), req_size);
> +	nreq = clamp(nreq, 4U, 64U);
> +	video->uvc_num_requests = nreq;
>  
>  	return 0;
>  }
> -- 
> 2.30.2
>