public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3] efi_loader: console: Correctly report modes
@ 2016-10-28  8:04 Emmanuel Vadot
  2016-10-28  9:33 ` Alexander Graf
  0 siblings, 1 reply; 3+ messages in thread
From: Emmanuel Vadot @ 2016-10-28  8:04 UTC (permalink / raw)
  To: u-boot

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 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@bidouilliste.com>
---
 lib/efi_loader/efi_console.c | 84 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 68 insertions(+), 16 deletions(-)

diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 2e0228c..eabf54f 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);
@@ -191,15 +220,36 @@ 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 = 2;
+		}
+
+		/*
+		 * 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_COUT_MODE_2].columns = n[2];
+			efi_cout_modes[EFI_COUT_MODE_2].rows = n[1];
+			efi_cout_modes[EFI_COUT_MODE_2].present = 1;
+			efi_con_mode.max_mode = EFI_MAX_COUT_MODE;
+			efi_con_mode.mode = EFI_COUT_MODE_2;
+		}
 	}
 
+	if (mode_number >= efi_con_mode.max_mode)
+		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 +260,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)
+		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(
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [U-Boot] [PATCH v3] efi_loader: console: Correctly report modes
  2016-10-28  8:04 [U-Boot] [PATCH v3] efi_loader: console: Correctly report modes Emmanuel Vadot
@ 2016-10-28  9:33 ` Alexander Graf
  2016-10-28 10:34   ` Emmanuel Vadot
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Graf @ 2016-10-28  9:33 UTC (permalink / raw)
  To: u-boot

On 10/28/2016 10:04 AM, 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 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@bidouilliste.com>
> ---
>   lib/efi_loader/efi_console.c | 84 +++++++++++++++++++++++++++++++++++---------
>   1 file changed, 68 insertions(+), 16 deletions(-)
>
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 2e0228c..eabf54f 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);
> @@ -191,15 +220,36 @@ 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) {

This is going to be very tedious to read for someone who isn't familiar 
with the return semantics of term_read_reply(). Please put n[x] into 
local col/row variables and use them in all conditionals afterwards.

> +			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 to the default one

I guess you want to say "set it as the currently selected mode"?

> +		 */
> +		if ((n[2] != 80 && n[1] != 25) || (n[2] != 80 && n[1] != 50)) {
> +			efi_cout_modes[EFI_COUT_MODE_2].columns = n[2];
> +			efi_cout_modes[EFI_COUT_MODE_2].rows = n[1];
> +			efi_cout_modes[EFI_COUT_MODE_2].present = 1;
> +			efi_con_mode.max_mode = EFI_MAX_COUT_MODE;
> +			efi_con_mode.mode = EFI_COUT_MODE_2;
> +		}
>   	}
>   
> +	if (mode_number >= efi_con_mode.max_mode)
> +		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 +260,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);

You're dropping the MODE_TEXT check - is that intentional?


Alex

>   
> -	return EFI_EXIT(EFI_UNSUPPORTED);
> +	if (mode_number > efi_con_mode.max_mode)
> +		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(

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [U-Boot] [PATCH v3] efi_loader: console: Correctly report modes
  2016-10-28  9:33 ` Alexander Graf
@ 2016-10-28 10:34   ` Emmanuel Vadot
  0 siblings, 0 replies; 3+ messages in thread
From: Emmanuel Vadot @ 2016-10-28 10:34 UTC (permalink / raw)
  To: u-boot

On Fri, 28 Oct 2016 11:33:19 +0200
Alexander Graf <agraf@suse.de> wrote:

> On 10/28/2016 10:04 AM, 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 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@bidouilliste.com>
> > ---
> >   lib/efi_loader/efi_console.c | 84 +++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 68 insertions(+), 16 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> > index 2e0228c..eabf54f 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);
> > @@ -191,15 +220,36 @@ 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) {
> 
> This is going to be very tedious to read for someone who isn't familiar 
> with the return semantics of term_read_reply(). Please put n[x] into 
> local col/row variables and use them in all conditionals afterwards.

 Will do.

> > +			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 to the default one
> 
> I guess you want to say "set it as the currently selected mode"?

 Yes, I'll change this.

> > +		 */
> > +		if ((n[2] != 80 && n[1] != 25) || (n[2] != 80 && n[1] != 50)) {
> > +			efi_cout_modes[EFI_COUT_MODE_2].columns = n[2];
> > +			efi_cout_modes[EFI_COUT_MODE_2].rows = n[1];
> > +			efi_cout_modes[EFI_COUT_MODE_2].present = 1;
> > +			efi_con_mode.max_mode = EFI_MAX_COUT_MODE;
> > +			efi_con_mode.mode = EFI_COUT_MODE_2;
> > +		}
> >   	}
> >   
> > +	if (mode_number >= efi_con_mode.max_mode)
> > +		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 +260,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);
> 
> You're dropping the MODE_TEXT check - is that intentional?
> 
> 
> Alex

 Yes.
 Set mode switch between cout modes, the supplied mode number just have
to exists.
 After re-reading the spec I see that the cursor needs to be
set to 0x0, I'll update that too.

> >   
> > -	return EFI_EXIT(EFI_UNSUPPORTED);
> > +	if (mode_number > efi_con_mode.max_mode)
> > +		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(


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-10-28 10:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-28  8:04 [U-Boot] [PATCH v3] efi_loader: console: Correctly report modes Emmanuel Vadot
2016-10-28  9:33 ` Alexander Graf
2016-10-28 10:34   ` Emmanuel Vadot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox