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

[Alexander Graf]

On 27/10/2016 18:11, 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 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 | 78 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 64 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 2e0228c..f4adc9e 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -9,11 +9,41 @@
>  #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_MAX_COUT_MODE 2
> +
> +struct cout_mode {
> +	unsigned long number;
> +	unsigned long columns;
> +	unsigned long rows;
> +	int present;
> +};
> +
> +static struct cout_mode efi_cout_modes[] = {
> +	/* EFI Mode 0 is 80x25 and always present */
> +	{
> +		.number = 0,
> +		.columns = 80,
> +		.rows = 25,
> +		.present = 1,
> +	},
> +	/* EFI Mode 1 is always 80x50 */
> +	{
> +		.number = 1,
> +		.columns = 80,
> +		.rows = 50,
> +		.present = 0,
> +	},
> +	/* Value are unknown until we query the console */
> +	{
> +		.number = 2,
> +		.columns = 0,
> +		.rows = 0,
> +		.present = 0,
> +	},
> +};
> +
>  const efi_guid_t efi_guid_console_control = CONSOLE_CONTROL_GUID;
>  
>  #define cESC '\x1b'
> @@ -56,6 +86,7 @@ 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,
>  	.mode = 0,
> @@ -140,12 +171,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 > efi_cout_modes[efi_con_mode.mode].columns) {

Have you sent this patch through checkpatch.pl? The line above is wider
than 80 characters, right?

Just create an interim variable (struct cout_mode *mode =
&efi_cout_modes[efi_con_mode]?) and use that instead. That makes the
code easier to read and keeps lines short :).

>  			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 > efi_cout_modes[efi_con_mode.mode].rows) {
> +			efi_con_mode.cursor_row = efi_cout_modes[efi_con_mode.mode].rows;
>  		}
>  	}
>  
> @@ -191,15 +222,32 @@ static efi_status_t EFIAPI efi_cout_query_mode(
>  			goto out;
>  		}
>  
> -		console_columns = n[2];
> -		console_rows = n[1];
> +		/* Test if we can have Mode 1 */
> +		if (n[2] >= 80 && n[1] >= 50) {
> +			efi_cout_modes[1].present = 1;
> +			efi_con_mode.max_mode = 1;
> +		}
> +
> +		/* Install our mode as mode 2 if it is different than mode 0 or 1 and set it to the default one */
> +		if ((n[2] != 80 && n[1] != 25) || (n[2] != 80 && n[1] != 50)) {
> +			efi_cout_modes[EFI_MAX_COUT_MODE].columns = n[2];
> +			efi_cout_modes[EFI_MAX_COUT_MODE].rows = n[1];
> +			efi_cout_modes[EFI_MAX_COUT_MODE].present = 1;
> +			efi_con_mode.max_mode = EFI_MAX_COUT_MODE;
> +			efi_con_mode.mode = EFI_MAX_COUT_MODE;

>From the EFI Spec:

Mode: Current Mode of the graphics device. Valid mode numbers are 0 to
MaxMode -1.

So EFI_MAX_COUT_MODE should be 3 and you want above, right? Just define
a new constant for the "special" mode (2).

> +		}
>  	}
>  
> +	if (mode_number > efi_con_mode.max_mode)

This needs to be >=.

> +		return EFI_EXIT(EFI_UNSUPPORTED);
> +	if (efi_cout_modes[mode_number].present != 1)
> +		return EFI_EXIT(EFI_UNSUPPORTED);
> +
>  out:
>  	if (columns)
> -		*columns = console_columns;
> +		*columns = efi_cout_modes[mode_number].columns;
>  	if (rows)
> -		*rows = console_rows;
> +		*rows = efi_cout_modes[mode_number].rows;
>  
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
> @@ -210,11 +258,13 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>  {
>  	EFI_ENTRY("%p, %ld", this, mode_number);
>  
> -	/* We only support text output for now */
> -	if (mode_number == EFI_CONSOLE_MODE_TEXT)
> -		return EFI_EXIT(EFI_SUCCESS);
>  
> -	return EFI_EXIT(EFI_UNSUPPORTED);
> +	if (mode_number > efi_con_mode.max_mode)

>=


Alex

> +		return EFI_EXIT(EFI_UNSUPPORTED);
> +
> +	efi_con_mode.mode = mode_number;
> +
> +	return EFI_EXIT(EFI_SUCCESS);
>  }
>  
>  static efi_status_t EFIAPI efi_cout_set_attribute(
>