[PATCH 0/4] EFI: console: improves

[PATCH 0/4] EFI: console: improves

[Artem Lapkin]

Problems with Linux Grub and U-Boot EFI console
- multiplexed console output scrambled
- slow refresh rate for big console sizes
- vidconsole incompatible with unicode

This patchset provides the following solutions:

1) Fixed detection of vidconsole from within a multiplexed stdout string.
As you know, a user can use a comma-separated list of devices to set
stdin, stdout and stderr. For example, "setenv stdout serial,vidconsole"
is a multiplexed string.

2) Multiplexed adaptation of the query_console_size() function;
automatically determine the minimal console area that will fit all
outputs properly.

3) Setup the max rows and columns limit for the EFI console output.

4) If EFI_CONSOLE_UTF_SAFE is enabled and vidconsole is active, unicode
characters will be replaced with "." for all console outputs. Vidconsole
does not support unicode output, and your console will suffer display
issues if EFI_CONSOLE_UTF_SAFE is disabled.

Artem Lapkin (4):
  EFI: console: query_vidconsole: multiplex adaptation
  EFI: console: query_console_size: multiplex adaptation
  EFI: console: max rows and cols user limit
  EFI: console: improve vidconsole unicode output

 lib/efi_loader/Kconfig       | 21 +++++++++++++++++++++
 lib/efi_loader/efi_console.c | 27 +++++++++++++++++++--------
 2 files changed, 40 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/4] EFI: console: query_vidconsole: multiplex adaptation

[Artem Lapkin]

Fixed detection of vidconsole from within a multiplexed stdout string. As
you know, a user can use a comma-separated list of devices to set stdin,
stdout and stderr. For example, "setenv stdout serial,vidconsole" is a
multiplexed string.

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 lib/efi_loader/efi_console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 3b012e1a66..2d03285f82 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -314,7 +314,7 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
 	struct udevice *dev;
 	struct vidconsole_priv *priv;
 
-	if (!stdout_name || strncmp(stdout_name, "vidconsole", 10))
+	if (!stdout_name || !strstr(stdout_name, "vidconsole"))
 		return -ENODEV;
 	stdout_dev = stdio_get_by_name("vidconsole");
 	if (!stdout_dev)
-- 
2.25.1

[PATCH 2/4] EFI: console: query_console_size: multiplex adaptation

[Artem Lapkin]

Multiplexed adaptation of the query_console_size() function;
automatically determine the minimal console area that will fit all
outputs properly.

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 lib/efi_loader/efi_console.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 3b012e1a66..ef5cf21bf7 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -340,14 +340,18 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
 static void query_console_size(void)
 {
 	int rows = 25, cols = 80;
-	int ret = -ENODEV;
 
-	if (IS_ENABLED(CONFIG_DM_VIDEO))
-		ret = query_vidconsole(&rows, &cols);
-	if (ret)
-		ret = query_console_serial(&rows, &cols);
-	if (ret)
+	if (IS_ENABLED(CONFIG_DM_VIDEO) &&
+	    !query_vidconsole(&rows, &cols)) {
+		int rows_serial, cols_serial;
+
+		if (!query_console_serial(&rows_serial, &cols_serial)) {
+			rows = min(rows, rows_serial);
+			cols = min(cols, cols_serial);
+		}
+	} else if (query_console_serial(&rows, &cols)) {
 		return;
+	}
 
 	/* Test if we can have Mode 1 */
 	if (cols >= 80 && rows >= 50) {
-- 
2.25.1

[PATCH 3/4] EFI: console: max rows and cols user limit

[Artem Lapkin]

Setup the max rows and columns limit for the EFI console output.

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 lib/efi_loader/Kconfig       | 12 ++++++++++++
 lib/efi_loader/efi_console.c |  5 +++++
 2 files changed, 17 insertions(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index dacc3b5881..7d00d6cde5 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -381,4 +381,16 @@ config EFI_ESRT
 	help
 	  Enabling this option creates the ESRT UEFI system table.
 
+config EFI_CONSOLE_MAX_ROWS
+	int "setup console max rows"
+	default 0
+	help
+	  Set console max rows limit or set to zero to disable limit.
+
+config EFI_CONSOLE_MAX_COLS
+	int "setup console max cols"
+	default 0
+	help
+	  Set console max rows limit or set to zero to disable limit.
+
 endif
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 847069212e..b5d79d788f 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -351,6 +351,11 @@ static void query_console_size(void)
 	} else if (query_console_serial(&rows, &cols))
 		return;
 
+	if (CONFIG_EFI_CONSOLE_MAX_ROWS > 0)
+		rows = min(rows, CONFIG_EFI_CONSOLE_MAX_ROWS);
+	if (CONFIG_EFI_CONSOLE_MAX_COLS > 0)
+		cols = min(cols, CONFIG_EFI_CONSOLE_MAX_COLS);
+
 	/* Test if we can have Mode 1 */
 	if (cols >= 80 && rows >= 50) {
 		efi_cout_modes[1].present = 1;
-- 
2.25.1

[PATCH 4/4] EFI: console: improve vidconsole unicode output

[Artem Lapkin]

If EFI_CONSOLE_UTF_SAFE is enabled and vidconsole is active, unicode
characters will be replaced with "." for all console outputs.

Vidconsole does not support unicode output, and your console will suffer
display issues if EFI_CONSOLE_UTF_SAFE is disabled.

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 lib/efi_loader/Kconfig       | 9 +++++++++
 lib/efi_loader/efi_console.c | 6 +++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 7d00d6cde5..886e0ce111 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -393,4 +393,13 @@ config EFI_CONSOLE_MAX_COLS
 	help
 	  Set console max rows limit or set to zero to disable limit.
 
+config EFI_CONSOLE_UTF_SAFE
+	bool "If vidconsole is active, unicode characters will be replaced with '.'"
+	default n
+	help
+	  If EFI_CONSOLE_UTF_SAFE is enabled and vidconsole is active, unicode
+	  characters will be replaced with "." for all console outputs.
+	  Vidconsole does not support unicode output, and your console will
+	  suffer display issues if EFI_CONSOLE_UTF_SAFE is disabled.
+
 endif
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index b5d79d788f..bd1c14995d 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -19,6 +19,8 @@
 #define EFI_COUT_MODE_2 2
 #define EFI_MAX_COUT_MODE 3
 
+static int vidconsole_active;
+
 struct cout_mode {
 	unsigned long columns;
 	unsigned long rows;
@@ -163,7 +165,8 @@ static efi_status_t EFIAPI efi_cout_output_string(
 	}
 	pos = buf;
 	utf16_utf8_strcpy(&pos, string);
-	fputs(stdout, buf);
+	fputs(stdout, IS_ENABLED(CONFIG_EFI_CONSOLE_UTF_SAFE) &&
+	      strlen(buf) > 1 && vidconsole_active ? "." : buf);
 	free(buf);
 
 	/*
@@ -327,6 +330,7 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
 		return -ENODEV;
 	*rows = priv->rows;
 	*cols = priv->cols;
+	vidconsole_active = 1;
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH 1/4] EFI: console: query_vidconsole: multiplex adaptation

[Heinrich Schuchardt]

On 8/4/21 12:22 PM, Artem Lapkin wrote:
> Fixed detection of vidconsole from within a multiplexed stdout string. As
> you know, a user can use a comma-separated list of devices to set stdin,
> stdout and stderr. For example, "setenv stdout serial,vidconsole" is a
> multiplexed string.
>
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---
>   lib/efi_loader/efi_console.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 3b012e1a66..2d03285f82 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -314,7 +314,7 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
>   	struct udevice *dev;
>   	struct vidconsole_priv *priv;
>
> -	if (!stdout_name || strncmp(stdout_name, "vidconsole", 10))
> +	if (!stdout_name || !strstr(stdout_name, "vidconsole"))

If stdout == "serial,vidconsole", serial is the primary output and we
want to determine the size of the console window from serial and not
from vidconsole.

So this change seems wrong.

Best regards

Heinrich

>   		return -ENODEV;
>   	stdout_dev = stdio_get_by_name("vidconsole");
>   	if (!stdout_dev)
>

Re: [PATCH 3/4] EFI: console: max rows and cols user limit

[Heinrich Schuchardt]

On 8/4/21 12:22 PM, Artem Lapkin wrote:
> Setup the max rows and columns limit for the EFI console output.

Why should a user set this up?

The size of serial console depends on the remote computers console
windows size.

The size of a video console depends on the attached monitor.

So we have to detect the size dynamically. Hardcoding it does not make
any sense.

Best regards

Heinrich

>
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---
>   lib/efi_loader/Kconfig       | 12 ++++++++++++
>   lib/efi_loader/efi_console.c |  5 +++++
>   2 files changed, 17 insertions(+)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index dacc3b5881..7d00d6cde5 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -381,4 +381,16 @@ config EFI_ESRT
>   	help
>   	  Enabling this option creates the ESRT UEFI system table.
>
> +config EFI_CONSOLE_MAX_ROWS
> +	int "setup console max rows"
> +	default 0
> +	help
> +	  Set console max rows limit or set to zero to disable limit.
> +
> +config EFI_CONSOLE_MAX_COLS
> +	int "setup console max cols"
> +	default 0
> +	help
> +	  Set console max rows limit or set to zero to disable limit.
> +
>   endif
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 847069212e..b5d79d788f 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -351,6 +351,11 @@ static void query_console_size(void)
>   	} else if (query_console_serial(&rows, &cols))
>   		return;
>
> +	if (CONFIG_EFI_CONSOLE_MAX_ROWS > 0)
> +		rows = min(rows, CONFIG_EFI_CONSOLE_MAX_ROWS);
> +	if (CONFIG_EFI_CONSOLE_MAX_COLS > 0)
> +		cols = min(cols, CONFIG_EFI_CONSOLE_MAX_COLS);
> +
>   	/* Test if we can have Mode 1 */
>   	if (cols >= 80 && rows >= 50) {
>   		efi_cout_modes[1].present = 1;
>

Re: [PATCH 4/4] EFI: console: improve vidconsole unicode output

[Heinrich Schuchardt]

On 8/4/21 12:22 PM, Artem Lapkin wrote:
> If EFI_CONSOLE_UTF_SAFE is enabled and vidconsole is active, unicode
> characters will be replaced with "." for all console outputs.

The TrueType console is meant to support Unicode including the special
characters used by GRUB like left and right triangle.

So this patch is based on a wrong assumption.

Best regards

Heinrich

>
> Vidconsole does not support unicode output, and your console will suffer
> display issues if EFI_CONSOLE_UTF_SAFE is disabled.
>
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---
>   lib/efi_loader/Kconfig       | 9 +++++++++
>   lib/efi_loader/efi_console.c | 6 +++++-
>   2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 7d00d6cde5..886e0ce111 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -393,4 +393,13 @@ config EFI_CONSOLE_MAX_COLS
>   	help
>   	  Set console max rows limit or set to zero to disable limit.
>
> +config EFI_CONSOLE_UTF_SAFE
> +	bool "If vidconsole is active, unicode characters will be replaced with '.'"
> +	default n
> +	help
> +	  If EFI_CONSOLE_UTF_SAFE is enabled and vidconsole is active, unicode
> +	  characters will be replaced with "." for all console outputs.
> +	  Vidconsole does not support unicode output, and your console will
> +	  suffer display issues if EFI_CONSOLE_UTF_SAFE is disabled.
> +
>   endif
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index b5d79d788f..bd1c14995d 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -19,6 +19,8 @@
>   #define EFI_COUT_MODE_2 2
>   #define EFI_MAX_COUT_MODE 3
>
> +static int vidconsole_active;
> +
>   struct cout_mode {
>   	unsigned long columns;
>   	unsigned long rows;
> @@ -163,7 +165,8 @@ static efi_status_t EFIAPI efi_cout_output_string(
>   	}
>   	pos = buf;
>   	utf16_utf8_strcpy(&pos, string);
> -	fputs(stdout, buf);
> +	fputs(stdout, IS_ENABLED(CONFIG_EFI_CONSOLE_UTF_SAFE) &&
> +	      strlen(buf) > 1 && vidconsole_active ? "." : buf);
>   	free(buf);
>
>   	/*
> @@ -327,6 +330,7 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
>   		return -ENODEV;
>   	*rows = priv->rows;
>   	*cols = priv->cols;
> +	vidconsole_active = 1;
>   	return 0;
>   }
>
>

Re: [PATCH 1/4] EFI: console: query_vidconsole: multiplex adaptation

[Art Nikpal]

> So this change seems wrong.
please check 3rd patch before
https://patchwork.ozlabs.org/project/uboot/patch/20210804102217.2419510-3-art@khadas.com/

i think its must be clarify problem with multiplex output configuration

CONFIG_CONSOLE_MUX=y
CONFIG_CONSOLE_NORMAL=y

please check more information and examples about this
+ https://dl.khadas.com/test/uboot_efi_examples/#readme
+ https://dl.khadas.com/test/uboot_efi_examples/media/01-EFI_console_example_normal_vidconsole_and_serial_as_is_without_changes_NOT_USABLE.png
+ https://dl.khadas.com/test/uboot_efi_examples/media/02-EFI_console_example_true_type_vidconsole_and_serial_as_is_NOT_USABLE.mp4
+ https://dl.khadas.com/test/uboot_efi_examples/media/02-EFI_console_example_true_type_vidconsole_and_serial_as_is_NOT_USABLE.png

On Thu, Aug 5, 2021 at 7:55 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 8/4/21 12:22 PM, Artem Lapkin wrote:
> > Fixed detection of vidconsole from within a multiplexed stdout string. As
> > you know, a user can use a comma-separated list of devices to set stdin,
> > stdout and stderr. For example, "setenv stdout serial,vidconsole" is a
> > multiplexed string.
> >
> > Signed-off-by: Artem Lapkin <art@khadas.com>
> > ---
> >   lib/efi_loader/efi_console.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> > index 3b012e1a66..2d03285f82 100644
> > --- a/lib/efi_loader/efi_console.c
> > +++ b/lib/efi_loader/efi_console.c
> > @@ -314,7 +314,7 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
> >       struct udevice *dev;
> >       struct vidconsole_priv *priv;
> >
> > -     if (!stdout_name || strncmp(stdout_name, "vidconsole", 10))
> > +     if (!stdout_name || !strstr(stdout_name, "vidconsole"))
>
> If stdout == "serial,vidconsole", serial is the primary output and we
> want to determine the size of the console window from serial and not
> from vidconsole.
>
> So this change seems wrong.
>
> Best regards
>
> Heinrich
>
> >               return -ENODEV;
> >       stdout_dev = stdio_get_by_name("vidconsole");
> >       if (!stdout_dev)
> >
>

Re: [PATCH 4/4] EFI: console: improve vidconsole unicode output

[Art Nikpal]

> The TrueType console is meant to support Unicode including the special
characters used by GRUB like left and right triangle.

Yes is meant but not works (on my side) ...

uboot TrueType vidconsole does not work properly as a unicode console
in my case and have other problems

CONFIG_CONSOLE_MUX=y
CONFIG_CONSOLE_NORMAL=y
CONFIG_CONSOLE_TRUETYPE=y
CONFIG_CONSOLE_TRUETYPE_SIZE=18
CONFIG_CONSOLE_TRUETYPE_NIMBUS=y

please check more information and examples about this

+ https://dl.khadas.com/test/uboot_efi_examples/#readme
+ https://dl.khadas.com/test/uboot_efi_examples/media/02-EFI_console_example_true_type_vidconsole_and_serial_as_is_NOT_USABLE.mp4
+ https://dl.khadas.com/test/uboot_efi_examples/media/02-EFI_console_example_true_type_vidconsole_and_serial_as_is_NOT_USABLE.png
+ https://dl.khadas.com/test/uboot_efi_examples/media/02-EFI_console_example_true_type_vidconsole_and_serial_as_is_NOT_USABLE_2.png

On Thu, Aug 5, 2021 at 8:01 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 8/4/21 12:22 PM, Artem Lapkin wrote:
> > If EFI_CONSOLE_UTF_SAFE is enabled and vidconsole is active, unicode
> > characters will be replaced with "." for all console outputs.
>
> The TrueType console is meant to support Unicode including the special
> characters used by GRUB like left and right triangle.
>
> So this patch is based on a wrong assumption.
>
> Best regards
>
> Heinrich
>
> >
> > Vidconsole does not support unicode output, and your console will suffer
> > display issues if EFI_CONSOLE_UTF_SAFE is disabled.
> >
> > Signed-off-by: Artem Lapkin <art@khadas.com>
> > ---
> >   lib/efi_loader/Kconfig       | 9 +++++++++
> >   lib/efi_loader/efi_console.c | 6 +++++-
> >   2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 7d00d6cde5..886e0ce111 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -393,4 +393,13 @@ config EFI_CONSOLE_MAX_COLS
> >       help
> >         Set console max rows limit or set to zero to disable limit.
> >
> > +config EFI_CONSOLE_UTF_SAFE
> > +     bool "If vidconsole is active, unicode characters will be replaced with '.'"
> > +     default n
> > +     help
> > +       If EFI_CONSOLE_UTF_SAFE is enabled and vidconsole is active, unicode
> > +       characters will be replaced with "." for all console outputs.
> > +       Vidconsole does not support unicode output, and your console will
> > +       suffer display issues if EFI_CONSOLE_UTF_SAFE is disabled.
> > +
> >   endif
> > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> > index b5d79d788f..bd1c14995d 100644
> > --- a/lib/efi_loader/efi_console.c
> > +++ b/lib/efi_loader/efi_console.c
> > @@ -19,6 +19,8 @@
> >   #define EFI_COUT_MODE_2 2
> >   #define EFI_MAX_COUT_MODE 3
> >
> > +static int vidconsole_active;
> > +
> >   struct cout_mode {
> >       unsigned long columns;
> >       unsigned long rows;
> > @@ -163,7 +165,8 @@ static efi_status_t EFIAPI efi_cout_output_string(
> >       }
> >       pos = buf;
> >       utf16_utf8_strcpy(&pos, string);
> > -     fputs(stdout, buf);
> > +     fputs(stdout, IS_ENABLED(CONFIG_EFI_CONSOLE_UTF_SAFE) &&
> > +           strlen(buf) > 1 && vidconsole_active ? "." : buf);
> >       free(buf);
> >
> >       /*
> > @@ -327,6 +330,7 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
> >               return -ENODEV;
> >       *rows = priv->rows;
> >       *cols = priv->cols;
> > +     vidconsole_active = 1;
> >       return 0;
> >   }
> >
> >
>

Re: [PATCH 3/4] EFI: console: max rows and cols user limit

[Art Nikpal]

> Why should a user set this up?

only if user need it! by default is 0 no limit

EFI console not usable for some cases: big screen sizes + mux with
serial console

for example

EFI console - fullscreen (1080P) vidconsole - SLUGGISH FRAMERATES AND NOT USABLE

CONFIG_CONSOLE_MUX=y
CONFIG_CONSOLE_NORMAL=y
CONFIG_EFI_CONSOLE_MAX_ROWS=0
CONFIG_EFI_CONSOLE_MAX_COLS=0

https://dl.khadas.com/test/uboot_efi_examples/media/07-EFI_console_example_normal_vidconsole_and_serial_multiplex_adaptation_utf_safe_SLOW_NOT_USABLE.mp4

EFI console - vidconsole with row and column limits - USABLE FRAMERATES

CONFIG_CONSOLE_MUX=y
CONFIG_CONSOLE_NORMAL=y
CONFIG_EFI_CONSOLE_MAX_ROWS=90
CONFIG_EFI_CONSOLE_MAX_COLS=25

https://dl.khadas.com/test/uboot_efi_examples/media/06-EFI_console_example_normal_vidconsole_and_serial_multiplex_adaptation_utf_safe_and_max_cols_90_limit_USABLE.mp4

Please check more information and examples about this

+ https://dl.khadas.com/test/uboot_efi_examples/#readme


On Thu, Aug 5, 2021 at 7:59 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 8/4/21 12:22 PM, Artem Lapkin wrote:
> > Setup the max rows and columns limit for the EFI console output.
>
> Why should a user set this up?
>
> The size of serial console depends on the remote computers console
> windows size.
>
> The size of a video console depends on the attached monitor.
>
> So we have to detect the size dynamically. Hardcoding it does not make
> any sense.
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Artem Lapkin <art@khadas.com>
> > ---
> >   lib/efi_loader/Kconfig       | 12 ++++++++++++
> >   lib/efi_loader/efi_console.c |  5 +++++
> >   2 files changed, 17 insertions(+)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index dacc3b5881..7d00d6cde5 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -381,4 +381,16 @@ config EFI_ESRT
> >       help
> >         Enabling this option creates the ESRT UEFI system table.
> >
> > +config EFI_CONSOLE_MAX_ROWS
> > +     int "setup console max rows"
> > +     default 0
> > +     help
> > +       Set console max rows limit or set to zero to disable limit.
> > +
> > +config EFI_CONSOLE_MAX_COLS
> > +     int "setup console max cols"
> > +     default 0
> > +     help
> > +       Set console max rows limit or set to zero to disable limit.
> > +
> >   endif
> > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> > index 847069212e..b5d79d788f 100644
> > --- a/lib/efi_loader/efi_console.c
> > +++ b/lib/efi_loader/efi_console.c
> > @@ -351,6 +351,11 @@ static void query_console_size(void)
> >       } else if (query_console_serial(&rows, &cols))
> >               return;
> >
> > +     if (CONFIG_EFI_CONSOLE_MAX_ROWS > 0)
> > +             rows = min(rows, CONFIG_EFI_CONSOLE_MAX_ROWS);
> > +     if (CONFIG_EFI_CONSOLE_MAX_COLS > 0)
> > +             cols = min(cols, CONFIG_EFI_CONSOLE_MAX_COLS);
> > +
> >       /* Test if we can have Mode 1 */
> >       if (cols >= 80 && rows >= 50) {
> >               efi_cout_modes[1].present = 1;
> >
>

Re: [PATCH 3/4] EFI: console: max rows and cols user limit

[Heinrich Schuchardt]

On 8/6/21 9:58 AM, Art Nikpal wrote:
>> Why should a user set this up?
>
> only if user need it! by default is 0 no limit
>
> EFI console not usable for some cases: big screen sizes + mux with
> serial console
>
> for example
>
> EFI console - fullscreen (1080P) vidconsole - SLUGGISH FRAMERATES AND NOT USABLE
>
> CONFIG_CONSOLE_MUX=y
> CONFIG_CONSOLE_NORMAL=y
> CONFIG_EFI_CONSOLE_MAX_ROWS=0
> CONFIG_EFI_CONSOLE_MAX_COLS=0
>
> https://dl.khadas.com/test/uboot_efi_examples/media/07-EFI_console_example_normal_vidconsole_and_serial_multiplex_adaptation_utf_safe_SLOW_NOT_USABLE.mp4
>
> EFI console - vidconsole with row and column limits - USABLE FRAMERATES
>
> CONFIG_CONSOLE_MUX=y
> CONFIG_CONSOLE_NORMAL=y
> CONFIG_EFI_CONSOLE_MAX_ROWS=90
> CONFIG_EFI_CONSOLE_MAX_COLS=25
>
> https://dl.khadas.com/test/uboot_efi_examples/media/06-EFI_console_example_normal_vidconsole_and_serial_multiplex_adaptation_utf_safe_and_max_cols_90_limit_USABLE.mp4
>
> Please check more information and examples about this
>
> + https://dl.khadas.com/test/uboot_efi_examples/#readme
>
>
> On Thu, Aug 5, 2021 at 7:59 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 8/4/21 12:22 PM, Artem Lapkin wrote:
>>> Setup the max rows and columns limit for the EFI console output.
>>
>> Why should a user set this up?
>>
>> The size of serial console depends on the remote computers console
>> windows size.
>>
>> The size of a video console depends on the attached monitor.
>>
>> So we have to detect the size dynamically. Hardcoding it does not make
>> any sense.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Signed-off-by: Artem Lapkin <art@khadas.com>
>>> ---
>>>    lib/efi_loader/Kconfig       | 12 ++++++++++++
>>>    lib/efi_loader/efi_console.c |  5 +++++
>>>    2 files changed, 17 insertions(+)
>>>
>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>> index dacc3b5881..7d00d6cde5 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -381,4 +381,16 @@ config EFI_ESRT
>>>        help
>>>          Enabling this option creates the ESRT UEFI system table.
>>>
>>> +config EFI_CONSOLE_MAX_ROWS
>>> +     int "setup console max rows"

This allows negative values.

Add an allowable range, e.g:

	range 0 65535

>>> +     default 0
>>> +     help
>>> +       Set console max rows limit or set to zero to disable limit.
>>> +
>>> +config EFI_CONSOLE_MAX_COLS
>>> +     int "setup console max cols"

	range 0 65535

>>> +     default 0
>>> +     help
>>> +       Set console max rows limit or set to zero to disable limit.
>>> +
>>>    endif
>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>> index 847069212e..b5d79d788f 100644
>>> --- a/lib/efi_loader/efi_console.c
>>> +++ b/lib/efi_loader/efi_console.c
>>> @@ -351,6 +351,11 @@ static void query_console_size(void)
>>>        } else if (query_console_serial(&rows, &cols))
>>>                return;
>>>
>>> +     if (CONFIG_EFI_CONSOLE_MAX_ROWS > 0)
>>> +             rows = min(rows, CONFIG_EFI_CONSOLE_MAX_ROWS);
>>> +     if (CONFIG_EFI_CONSOLE_MAX_COLS > 0)
>>> +             cols = min(cols, CONFIG_EFI_CONSOLE_MAX_COLS);
>>> +

We should not neither call query_vidconsole() nor query_console_serial()
if both CONFIG_EFI_CONSOLE_MAX_ROWS and CONFIG_EFI_CONSOLE_MAX_COLS are set.

The UEFI specification requires that a device providing the
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL must at least support a mode with size
80x25. But some LCD screens are smaller. So if
CONFIG_EFI_CONSOLE_MAX_ROWS or CONFIG_EFI_CONSOLE_MAX_COLS doesn't fit
into 80x25, we should use CONFIG_EFI_CONSOLE_MAX_ROWS and
CONFIG_EFI_CONSOLE_MAX_COLS for mode 0.

Best regards

Heinrich

>>>        /* Test if we can have Mode 1 */
>>>        if (cols >= 80 && rows >= 50) {
>>>                efi_cout_modes[1].present = 1;
>>>
>>