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

[Mark Kettenis]

> From: Thomas Watson <twatson52 at icloud.com>
> Date: Tue, 21 Dec 2021 19:36:16 -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>
> ---

I can confirm that plugging in a usb keyboard makes loading a kernel
annoyingly slow when booting through EFI.  And this looks like a
reasonable approach to fix this to me.  Minor nit: I think you should
reflow the comment you're adding such that it fits within the standard
80 character line limit.


> 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..0131b7dfab 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.17.1
> 
>