Re: [PATCH v2 5/6] efi/console: Do not set text-mode until we actually need it

[Daniel Kiper]

On Tue, Apr 07, 2020 at 11:03:50AM +0200, Javier Martinez Canillas wrote:
> From: Hans de Goede <address@hidden>
>
> If we're running with a hidden menu we may never need text mode, so do not
> change the video-mode to text until we actually need it.
>
> This allows to boot a machine without unnecessary graphical transitions and
> provide a seamless boot experience to users.
>
> Signed-off-by: Hans de Goede <address@hidden>
> Signed-off-by: Javier Martinez Canillas <address@hidden>
> ---
>
> Changes in v2: None
>
>  grub-core/term/efi/console.c | 74 +++++++++++++++++++++++-------------
>  include/grub/term.h          |  4 +-
>  2 files changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
> index 9c32ca60a1d..f7c9b37e95d 100644
> --- a/grub-core/term/efi/console.c
> +++ b/grub-core/term/efi/console.c
> @@ -24,6 +24,16 @@
>  #include <grub/efi/api.h>
>  #include <grub/efi/console.h>
>
> +typedef enum {
> +    GRUB_TEXT_MODE_UNDEFINED = -1,
> +    GRUB_TEXT_MODE_UNAVAILABLE = 0,
> +    GRUB_TEXT_MODE_AVAILABLE
> +}
> +grub_text_mode;
> +
> +static grub_text_mode text_mode = GRUB_TEXT_MODE_UNDEFINED;
> +static grub_term_color_state text_colorstate = GRUB_TERM_COLOR_UNDEFINED;
> +
>  static grub_uint32_t
>  map_char (grub_uint32_t c)
>  {
> @@ -106,15 +116,39 @@ grub_console_setcursor (struct grub_term_output *term 
> __attribute__ ((unused)),
>    efi_call_2 (o->enable_cursor, o, on);
>  }
>
> +static grub_err_t
> +grub_prepare_for_text_output (struct grub_term_output *term)
> +{
> +  if (grub_efi_is_finished)
> +    return GRUB_ERR_BAD_DEVICE;
> +
> +  if (text_mode != GRUB_TEXT_MODE_UNDEFINED)
> +    return text_mode ? 0 : GRUB_ERR_BAD_DEVICE;

Nit, s/0/GRUB_ERR_NONE/...

> +
> +  if (! grub_efi_set_text_mode (1))
> +    {
> +      /* This really should never happen */
> +      grub_error (GRUB_ERR_BAD_DEVICE, "cannot set text mode");
> +      text_mode = GRUB_TEXT_MODE_UNAVAILABLE;
> +      return GRUB_ERR_BAD_DEVICE;
> +    }
> +
> +  grub_console_setcursor (term, 1);
> +  if (text_colorstate != GRUB_TERM_COLOR_UNDEFINED)
> +    grub_console_setcolorstate (term, text_colorstate);
> +  text_mode = GRUB_TEXT_MODE_AVAILABLE;
> +  return 0;

Nit, s/0/GRUB_ERR_NONE/...

> +}
> +
>  static void
> -grub_console_putchar (struct grub_term_output *term __attribute__ ((unused)),
> +grub_console_putchar (struct grub_term_output *term,
>                     const struct grub_unicode_glyph *c)
>  {
>    grub_efi_char16_t str[2 + 30];
>    grub_efi_simple_text_output_interface_t *o;
>    unsigned i, j;
>
> -  if (grub_efi_is_finished)
> +  if (grub_prepare_for_text_output (term))

!= GRUB_ERR_NONE and below...

Otherwise LGTM...

Daniel