[PATCH v2] console: usb: kbd: Limit poll frequency to improve performance

Mark Kettenis mark.kettenis at xs4all.nl
Sun Jan 2 17:25:33 CET 2022


> From: Thomas Watson <twatson52 at icloud.com>
> Date: Wed, 22 Dec 2021 16:51:26 -0600
> 
> Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
> 30-40ms to run. The exact time is dependent on the polling interval the
> keyboard requests in its descriptor, and likely cannot be significantly
> reduced without major rework to the XHCI driver.
> 
> The U-Boot EFI console service sets a timer to poll the keyboard every 5
> microseconds, and this timer is checked every time a block is read off
> disk. The net effect is that, on my system, loading a ~40MiB kernel and
> initrd takes about 62 seconds with a slower keyboard and 53 seconds
> with a faster one, with the vast majority of the time spent polling the
> keyboard.
> 
> To solve this problem, this patch adds a 20ms delay between consecutive
> calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
> total loading time to under half a second for both keyboards, and does
> not impact the perceived keystroke latency.
> 
> Signed-off-by: Thomas Watson <twatson52 at icloud.com>
> ---
> This revision wraps the comment at 80 characters and also should not
> have been corrupted by my e-mail client.

This fixes a *major* annoyance with using the EFI boot path while
having a USB keyboard plugged in, and the approach seems entirely
reasonable to me and addresses the concerns raised by Heinrich in an
earlier attempt to fix this issue:

https://patchwork.ozlabs.org/project/uboot/patch/20210708082250.11833-1-mchang@suse.com/


Tested-by: Mark Kettenis <kettenis at openbsd.org>
Reviewed-by: Mark Kettenis <kettenis at openbsd.org>

> 
>  common/usb_kbd.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index afad260d3d..5408c821b4 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -118,7 +118,7 @@ struct usb_kbd_pdata {
>  extern int __maybe_unused net_busy_flag;
>  
>  /* The period of time between two calls of usb_kbd_testc(). */
> -static unsigned long __maybe_unused kbd_testc_tms;
> +static unsigned long kbd_testc_tms;
>  
>  /* Puts character in the queue and sets up the in and out pointer. */
>  static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c)
> @@ -394,21 +394,31 @@ static int usb_kbd_testc(struct stdio_dev *sdev)
>  	struct usb_device *usb_kbd_dev;
>  	struct usb_kbd_pdata *data;
>  
> +	/*
> +	 * Polling the keyboard for an event can take dozens of milliseconds.
> +	 * Add a delay between polls to avoid blocking activity which polls
> +	 * rapidly, like the UEFI console timer.
> +	 */
> +	unsigned long poll_delay = CONFIG_SYS_HZ / 50;
> +
>  #ifdef CONFIG_CMD_NET
>  	/*
>  	 * If net_busy_flag is 1, NET transfer is running,
>  	 * then we check key-pressed every second (first check may be
>  	 * less than 1 second) to improve TFTP booting performance.
>  	 */
> -	if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ))
> -		return 0;
> -	kbd_testc_tms = get_timer(0);
> +	if (net_busy_flag)
> +		poll_delay = CONFIG_SYS_HZ;
>  #endif
> +
>  	dev = stdio_get_by_name(sdev->name);
>  	usb_kbd_dev = (struct usb_device *)dev->priv;
>  	data = usb_kbd_dev->privptr;
>  
> -	usb_kbd_poll_for_event(usb_kbd_dev);
> +	if (get_timer(kbd_testc_tms) >= poll_delay) {
> +		usb_kbd_poll_for_event(usb_kbd_dev);
> +		kbd_testc_tms = get_timer(0);
> +	}
>  
>  	return !(data->usb_in_pointer == data->usb_out_pointer);
>  }
> -- 
> 2.31.1
> 
> 


More information about the U-Boot mailing list