[U-Boot] [PATCH v4] efi_loader: console: Correctly report modes

[Alexander Graf]

On 28/10/2016 17:35, Emmanuel Vadot wrote:
> Add support for EFI console modes.
> Mode 0 is always 80x25 and present by EFI specification.
> Mode 1 is always 80x50 and not mandatory.
> Mode 2 and above is freely usable.
>
> If the terminal can handle mode 1, we mark it as supported.
> If the terminal size is greater than mode 0 and different than mode 1,
> we install it as mode 2.
>
> Modes can be switch with cout_set_mode.
>
> Changes in V4:
>  Reset cursor positon on mode switch
>  Use local variables in console query code
>
> Changes in V3:
>  Valid mode are 0 to EFIMode-1
>  Fix style
>
> Changes in V2:
>  Add mode switch
>  Report only the modes that we support
>
> Signed-off-by: Emmanuel Vadot <manu at bidouilliste.com>
> ---
>  lib/efi_loader/efi_console.c | 90 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 74 insertions(+), 16 deletions(-)
>
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 2e0228c..c22eb3e 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -9,11 +9,38 @@
>  #include <common.h>
>  #include <efi_loader.h>
>
> -/* If we can't determine the console size, default to 80x24 */
> -static int console_columns = 80;
> -static int console_rows = 24;
>  static bool console_size_queried;
>
> +#define EFI_COUT_MODE_2 2
> +#define EFI_MAX_COUT_MODE 3
> +
> +struct cout_mode {
> +	unsigned long columns;
> +	unsigned long rows;
> +	int present;
> +};
> +
> +static struct cout_mode efi_cout_modes[] = {
> +	/* EFI Mode 0 is 80x25 and always present */
> +	{
> +		.columns = 80,
> +		.rows = 25,
> +		.present = 1,
> +	},
> +	/* EFI Mode 1 is always 80x50 */
> +	{
> +		.columns = 80,
> +		.rows = 50,
> +		.present = 0,
> +	},
> +	/* Value are unknown until we query the console */
> +	{
> +		.columns = 0,
> +		.rows = 0,
> +		.present = 0,
> +	},
> +};
> +
>  const efi_guid_t efi_guid_console_control = CONSOLE_CONTROL_GUID;
>
>  #define cESC '\x1b'
> @@ -56,8 +83,9 @@ const struct efi_console_control_protocol efi_console_control = {
>  	.lock_std_in = efi_cin_lock_std_in,
>  };
>
> +/* Default to mode 0 */
>  static struct simple_text_output_mode efi_con_mode = {
> -	.max_mode = 0,
> +	.max_mode = 1,
>  	.mode = 0,
>  	.attribute = 0,
>  	.cursor_column = 0,
> @@ -131,8 +159,10 @@ static efi_status_t EFIAPI efi_cout_output_string(
>  			struct efi_simple_text_output_protocol *this,
>  			const unsigned short *string)
>  {
> +	struct cout_mode *mode;
>  	u16 ch;
>
> +	mode = &efi_cout_modes[efi_con_mode.mode];
>  	EFI_ENTRY("%p, %p", this, string);
>  	for (;(ch = *string); string++) {
>  		print_unicode_in_utf8(ch);
> @@ -140,13 +170,12 @@ static efi_status_t EFIAPI efi_cout_output_string(
>  		if (ch == '\n') {
>  			efi_con_mode.cursor_column = 1;
>  			efi_con_mode.cursor_row++;
> -		} else if (efi_con_mode.cursor_column > console_columns) {
> +		} else if (efi_con_mode.cursor_column > mode->columns) {
>  			efi_con_mode.cursor_column = 1;
>  			efi_con_mode.cursor_row++;
>  		}
> -		if (efi_con_mode.cursor_row > console_rows) {
> -			efi_con_mode.cursor_row = console_rows;
> -		}
> +		if (efi_con_mode.cursor_row > mode->rows)
> +			efi_con_mode.cursor_row = mode->rows;
>  	}
>
>  	return EFI_EXIT(EFI_SUCCESS);
> @@ -170,6 +199,8 @@ static efi_status_t EFIAPI efi_cout_query_mode(
>  	if (!console_size_queried) {
>  		/* Ask the terminal about its size */
>  		int n[3];
> +		int cols;
> +		int rows;
>  		u64 timeout;
>
>  		console_size_queried = true;
> @@ -191,15 +222,38 @@ static efi_status_t EFIAPI efi_cout_query_mode(
>  			goto out;
>  		}
>
> -		console_columns = n[2];
> -		console_rows = n[1];
> +		cols = n[2];
> +		rows = n[1];

Please add a new line here :).

> +		/* Test if we can have Mode 1 */
> +		if (cols >= 80 && rows >= 50) {
> +			efi_cout_modes[1].present = 1;
> +			efi_con_mode.max_mode = 2;
> +		}
> +
> +		/*
> +		 * Install our mode as mode 2 if it is different
> +		 * than mode 0 or 1 and set it  as the currently selected mode
> +		 */
> +		if ((cols != 80 && rows != 25) || (cols != 80 && rows != 50)) {

This equation is always true, no?

cols = 80, rows = 25:

   if (true || false) -> true

cols = 80, rows = 50:

   if (false || true) -> true

cols = 10 rows = 10:

   if (true || true) -> true

Basically what you want is

   if (cols != 80) && (rows != 25) && (rows != 50))

or if you want extra readability points, something like

   static bool cout_mode_matches(struct cout_mode *mode, rows, cols)
   {
     if (!mode->present)
       return false;

     return ((mode->rows == rows) && (mode->columns == cols));
   }

   ...

   if (!cout_mode_matches(&efi_cout_modes[0], rows, cols) &&
       !cout_mode_matches(&efi_cout_modes[1], rows, cols)) {
     ...
   }


Alex