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

[Peter Robinson]

On Wed, Dec 22, 2021 at 11:16 PM Thomas Watson <twatson52 at icloud.com> wrote:
>
> 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>
Tested-by: Peter Robinson <pbrobinson at gmail.com>

Tested across a handful of devices booting Fedora via UEFI and LGTM.
> ---
> This revision wraps the comment at 80 characters and also should not
> have been corrupted by my e-mail client.
>
>  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
>