[PATCH 0/2] Add efitextmode command

[PATCH 0/2] Add efitextmode command

[Glenn Washburn]

This patch add the efitextmode command which is used for showing all
available text output modes and setting a specific mode. Its basically the
equivalent of the EFI Shell's "mode" command and its output looks similar.
The main difference is that its shows the mode number in the listing and
takes a mode number when setting the mode. As a convenience it can take
arguments "min" or "max" to set to the minimum or maximum mode.

Glenn

Glenn Washburn (2):
  efi: Add efitextmode command for getting/setting the text mode
    resolution
  docs: Add documentation for the efitextmode command

 docs/grub.texi                       |  23 ++++++
 grub-core/Makefile.core.def          |   6 ++
 grub-core/commands/efi/efitextmode.c | 118 +++++++++++++++++++++++++++
 3 files changed, 147 insertions(+)
 create mode 100644 grub-core/commands/efi/efitextmode.c

-- 
2.34.1

[PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution

[Glenn Washburn]

This command is meant to behave similarly to the 'mode' command of the EFI
Shell application. One difference is that to set the mode the mode number
is given, not the rows and columns of the desired mode. Also supported are
the arguments "min" and "max", which set the mode to the minimum and
maximum mode respectively as calculated by the columns * rows of that mode.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/Makefile.core.def          |   6 ++
 grub-core/commands/efi/efitextmode.c | 118 +++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)
 create mode 100644 grub-core/commands/efi/efitextmode.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 726f51be7..b22e48f0f 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -813,6 +813,12 @@ module = {
   enable = efi;
 };
 
+module = {
+  name = efitextmode;
+  efi = commands/efi/efitextmode.c;
+  enable = efi;
+};
+
 module = {
   name = blocklist;
   common = commands/blocklist.c;
diff --git a/grub-core/commands/efi/efitextmode.c b/grub-core/commands/efi/efitextmode.c
new file mode 100644
index 000000000..fb72aa6f3
--- /dev/null
+++ b/grub-core/commands/efi/efitextmode.c
@@ -0,0 +1,118 @@
+/* efitextmode.c - command to get/set text mode resolution */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2003,2005,2006,2007,2009  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/dl.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/command.h>
+#include <grub/i18n.h>
+#include <grub/efi/efi.h>
+#include <grub/efi/api.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static grub_err_t
+grub_cmd_efitextmode (grub_command_t cmd __attribute__ ((unused)),
+		      int argc, char **args)
+{
+  grub_efi_simple_text_output_interface_t *o;
+  unsigned long mode;
+  const char *p = NULL;
+  grub_efi_status_t status;
+  grub_efi_uintn_t columns, rows;
+  grub_efi_int32_t i;
+
+  if (argc > 1)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("at most one argument expected"));
+
+  o = grub_efi_system_table->con_out;
+
+  if (argc == 1)
+    {
+      if (grub_strcmp (args[0], "min") == 0)
+	mode = 0;
+      else if (grub_strcmp (args[0], "max") == 0)
+	mode = o->mode->max_mode - 1;
+      else
+	{
+	  mode = grub_strtoul (args[0], &p, 0);
+
+	  if (grub_errno != GRUB_ERR_NONE)
+	    return grub_errno;
+
+	  if (*args[0] == '\0' || *p != '\0')
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			       N_("non-numeric or invalid mode `%s'"),
+			       args[0]);
+	}
+
+      if (mode < (unsigned long) o->mode->max_mode)
+	{
+	  if (mode != (unsigned long) o->mode->mode)
+	    {
+	      status = efi_call_2 (o->set_mode, o, (grub_efi_int32_t) mode);
+	      if (status == GRUB_EFI_SUCCESS)
+		;
+	      else if (status == GRUB_EFI_DEVICE_ERROR)
+		return grub_error (GRUB_ERR_BAD_DEVICE,
+				   N_("device error: could not set requested"
+				      " mode"));
+	      else if (status == GRUB_EFI_UNSUPPORTED)
+		return grub_error (GRUB_ERR_OUT_OF_RANGE,
+				   N_("invalid mode: number not valid"));
+	      else
+		return grub_error (GRUB_ERR_BAD_OS,
+				   N_("unexpected EFI error number: `%u'"),
+				   (unsigned) status);
+	    }
+	}
+      else
+	return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			   N_("invalid mode: `%lu' is greater than maximum"
+			      " mode `%lu'"),
+			   mode, (unsigned long) o->mode->max_mode);
+    }
+
+  if (argc == 0)
+    {
+      grub_printf_ (N_("Available modes for console output device.\n"));
+
+      for (i=0; i < o->mode->max_mode; i++)
+	if (GRUB_EFI_SUCCESS == efi_call_4 (o->query_mode, o, i,
+					    &columns, &rows))
+	  grub_printf_ (N_(" [%lu]  Col %5u Row %5u %c\n"),
+			(unsigned long) i, (unsigned) columns, (unsigned) rows,
+			(i == o->mode->mode) ? '*' : ' ');
+    }
+
+  return GRUB_ERR_NONE;
+}
+
+static grub_command_t cmd;
+\f
+GRUB_MOD_INIT(cmp)
+{
+  cmd = grub_register_command ("efitextmode", grub_cmd_efitextmode,
+			       N_("[min|max|mode_num]"), N_("Get or set EFI text mode."));
+}
+
+GRUB_MOD_FINI(cmp)
+{
+  grub_unregister_command (cmd);
+}
-- 
2.34.1

Re: [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution

[Paul Menzel]

Dear Glenn,


Thank you for the patch.

Am 12.05.22 um 05:07 schrieb Glenn Washburn:
> This command is meant to behave similarly to the 'mode' command of the EFI
> Shell application. One difference is that to set the mode the mode number
> is given, not the rows and columns of the desired mode. Also supported are
> the arguments "min" and "max", which set the mode to the minimum and
> maximum mode respectively as calculated by the columns * rows of that mode.

Did you test this also with QEMU and OVMF?


Kind regards,

Paul


> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>   grub-core/Makefile.core.def          |   6 ++
>   grub-core/commands/efi/efitextmode.c | 118 +++++++++++++++++++++++++++
>   2 files changed, 124 insertions(+)
>   create mode 100644 grub-core/commands/efi/efitextmode.c
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 726f51be7..b22e48f0f 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -813,6 +813,12 @@ module = {
>     enable = efi;
>   };
>   
> +module = {
> +  name = efitextmode;
> +  efi = commands/efi/efitextmode.c;
> +  enable = efi;
> +};
> +
>   module = {
>     name = blocklist;
>     common = commands/blocklist.c;
> diff --git a/grub-core/commands/efi/efitextmode.c b/grub-core/commands/efi/efitextmode.c
> new file mode 100644
> index 000000000..fb72aa6f3
> --- /dev/null
> +++ b/grub-core/commands/efi/efitextmode.c
> @@ -0,0 +1,118 @@
> +/* efitextmode.c - command to get/set text mode resolution */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2003,2005,2006,2007,2009  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/dl.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/command.h>
> +#include <grub/i18n.h>
> +#include <grub/efi/efi.h>
> +#include <grub/efi/api.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static grub_err_t
> +grub_cmd_efitextmode (grub_command_t cmd __attribute__ ((unused)),
> +		      int argc, char **args)
> +{
> +  grub_efi_simple_text_output_interface_t *o;
> +  unsigned long mode;
> +  const char *p = NULL;
> +  grub_efi_status_t status;
> +  grub_efi_uintn_t columns, rows;
> +  grub_efi_int32_t i;
> +
> +  if (argc > 1)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("at most one argument expected"));
> +
> +  o = grub_efi_system_table->con_out;
> +
> +  if (argc == 1)
> +    {
> +      if (grub_strcmp (args[0], "min") == 0)
> +	mode = 0;
> +      else if (grub_strcmp (args[0], "max") == 0)
> +	mode = o->mode->max_mode - 1;
> +      else
> +	{
> +	  mode = grub_strtoul (args[0], &p, 0);
> +
> +	  if (grub_errno != GRUB_ERR_NONE)
> +	    return grub_errno;
> +
> +	  if (*args[0] == '\0' || *p != '\0')
> +	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +			       N_("non-numeric or invalid mode `%s'"),
> +			       args[0]);
> +	}
> +
> +      if (mode < (unsigned long) o->mode->max_mode)
> +	{
> +	  if (mode != (unsigned long) o->mode->mode)
> +	    {
> +	      status = efi_call_2 (o->set_mode, o, (grub_efi_int32_t) mode);
> +	      if (status == GRUB_EFI_SUCCESS)
> +		;
> +	      else if (status == GRUB_EFI_DEVICE_ERROR)
> +		return grub_error (GRUB_ERR_BAD_DEVICE,
> +				   N_("device error: could not set requested"
> +				      " mode"));
> +	      else if (status == GRUB_EFI_UNSUPPORTED)
> +		return grub_error (GRUB_ERR_OUT_OF_RANGE,
> +				   N_("invalid mode: number not valid"));
> +	      else
> +		return grub_error (GRUB_ERR_BAD_OS,
> +				   N_("unexpected EFI error number: `%u'"),
> +				   (unsigned) status);
> +	    }
> +	}
> +      else
> +	return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +			   N_("invalid mode: `%lu' is greater than maximum"
> +			      " mode `%lu'"),
> +			   mode, (unsigned long) o->mode->max_mode);
> +    }
> +
> +  if (argc == 0)
> +    {
> +      grub_printf_ (N_("Available modes for console output device.\n"));
> +
> +      for (i=0; i < o->mode->max_mode; i++)
> +	if (GRUB_EFI_SUCCESS == efi_call_4 (o->query_mode, o, i,
> +					    &columns, &rows))
> +	  grub_printf_ (N_(" [%lu]  Col %5u Row %5u %c\n"),
> +			(unsigned long) i, (unsigned) columns, (unsigned) rows,
> +			(i == o->mode->mode) ? '*' : ' ');
> +    }
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_command_t cmd;
> +\f
> +GRUB_MOD_INIT(cmp)
> +{
> +  cmd = grub_register_command ("efitextmode", grub_cmd_efitextmode,
> +			       N_("[min|max|mode_num]"), N_("Get or set EFI text mode."));
> +}
> +
> +GRUB_MOD_FINI(cmp)
> +{
> +  grub_unregister_command (cmd);
> +}

Re: [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution

[Glenn Washburn]

Hi Paul,

On Thu, 12 May 2022 08:10:56 +0200
Paul Menzel <pmenzel@molgen.mpg.de> wrote:

> Dear Glenn,
> 
> 
> Thank you for the patch.
> 
> Am 12.05.22 um 05:07 schrieb Glenn Washburn:
> > This command is meant to behave similarly to the 'mode' command of the EFI
> > Shell application. One difference is that to set the mode the mode number
> > is given, not the rows and columns of the desired mode. Also supported are
> > the arguments "min" and "max", which set the mode to the minimum and
> > maximum mode respectively as calculated by the columns * rows of that mode.
> 
> Did you test this also with QEMU and OVMF?

Yes, although with QEMU with -nographic and the output going through
serial, though not using GRUB's serial terminal. So its still using the
efi console. Is there a reason you're asking? Something not working as
expected?

Glenn

> 
> 
> Kind regards,
> 
> Paul
> 
> 
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >   grub-core/Makefile.core.def          |   6 ++
> >   grub-core/commands/efi/efitextmode.c | 118 +++++++++++++++++++++++++++
> >   2 files changed, 124 insertions(+)
> >   create mode 100644 grub-core/commands/efi/efitextmode.c
> > 
> > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> > index 726f51be7..b22e48f0f 100644
> > --- a/grub-core/Makefile.core.def
> > +++ b/grub-core/Makefile.core.def
> > @@ -813,6 +813,12 @@ module = {
> >     enable = efi;
> >   };
> >   
> > +module = {
> > +  name = efitextmode;
> > +  efi = commands/efi/efitextmode.c;
> > +  enable = efi;
> > +};
> > +
> >   module = {
> >     name = blocklist;
> >     common = commands/blocklist.c;
> > diff --git a/grub-core/commands/efi/efitextmode.c b/grub-core/commands/efi/efitextmode.c
> > new file mode 100644
> > index 000000000..fb72aa6f3
> > --- /dev/null
> > +++ b/grub-core/commands/efi/efitextmode.c
> > @@ -0,0 +1,118 @@
> > +/* efitextmode.c - command to get/set text mode resolution */
> > +/*
> > + *  GRUB  --  GRand Unified Bootloader
> > + *  Copyright (C) 2003,2005,2006,2007,2009  Free Software Foundation, Inc.
> > + *
> > + *  GRUB is free software: you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License as published by
> > + *  the Free Software Foundation, either version 3 of the License, or
> > + *  (at your option) any later version.
> > + *
> > + *  GRUB is distributed in the hope that it will be useful,
> > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *  GNU General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <grub/dl.h>
> > +#include <grub/misc.h>
> > +#include <grub/mm.h>
> > +#include <grub/command.h>
> > +#include <grub/i18n.h>
> > +#include <grub/efi/efi.h>
> > +#include <grub/efi/api.h>
> > +
> > +GRUB_MOD_LICENSE ("GPLv3+");
> > +
> > +static grub_err_t
> > +grub_cmd_efitextmode (grub_command_t cmd __attribute__ ((unused)),
> > +		      int argc, char **args)
> > +{
> > +  grub_efi_simple_text_output_interface_t *o;
> > +  unsigned long mode;
> > +  const char *p = NULL;
> > +  grub_efi_status_t status;
> > +  grub_efi_uintn_t columns, rows;
> > +  grub_efi_int32_t i;
> > +
> > +  if (argc > 1)
> > +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("at most one argument expected"));
> > +
> > +  o = grub_efi_system_table->con_out;
> > +
> > +  if (argc == 1)
> > +    {
> > +      if (grub_strcmp (args[0], "min") == 0)
> > +	mode = 0;
> > +      else if (grub_strcmp (args[0], "max") == 0)
> > +	mode = o->mode->max_mode - 1;
> > +      else
> > +	{
> > +	  mode = grub_strtoul (args[0], &p, 0);
> > +
> > +	  if (grub_errno != GRUB_ERR_NONE)
> > +	    return grub_errno;
> > +
> > +	  if (*args[0] == '\0' || *p != '\0')
> > +	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > +			       N_("non-numeric or invalid mode `%s'"),
> > +			       args[0]);
> > +	}
> > +
> > +      if (mode < (unsigned long) o->mode->max_mode)
> > +	{
> > +	  if (mode != (unsigned long) o->mode->mode)
> > +	    {
> > +	      status = efi_call_2 (o->set_mode, o, (grub_efi_int32_t) mode);
> > +	      if (status == GRUB_EFI_SUCCESS)
> > +		;
> > +	      else if (status == GRUB_EFI_DEVICE_ERROR)
> > +		return grub_error (GRUB_ERR_BAD_DEVICE,
> > +				   N_("device error: could not set requested"
> > +				      " mode"));
> > +	      else if (status == GRUB_EFI_UNSUPPORTED)
> > +		return grub_error (GRUB_ERR_OUT_OF_RANGE,
> > +				   N_("invalid mode: number not valid"));
> > +	      else
> > +		return grub_error (GRUB_ERR_BAD_OS,
> > +				   N_("unexpected EFI error number: `%u'"),
> > +				   (unsigned) status);
> > +	    }
> > +	}
> > +      else
> > +	return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > +			   N_("invalid mode: `%lu' is greater than maximum"
> > +			      " mode `%lu'"),
> > +			   mode, (unsigned long) o->mode->max_mode);
> > +    }
> > +
> > +  if (argc == 0)
> > +    {
> > +      grub_printf_ (N_("Available modes for console output device.\n"));
> > +
> > +      for (i=0; i < o->mode->max_mode; i++)
> > +	if (GRUB_EFI_SUCCESS == efi_call_4 (o->query_mode, o, i,
> > +					    &columns, &rows))
> > +	  grub_printf_ (N_(" [%lu]  Col %5u Row %5u %c\n"),
> > +			(unsigned long) i, (unsigned) columns, (unsigned) rows,
> > +			(i == o->mode->mode) ? '*' : ' ');
> > +    }
> > +
> > +  return GRUB_ERR_NONE;
> > +}
> > +
> > +static grub_command_t cmd;
> > +\f
> > +GRUB_MOD_INIT(cmp)
> > +{
> > +  cmd = grub_register_command ("efitextmode", grub_cmd_efitextmode,
> > +			       N_("[min|max|mode_num]"), N_("Get or set EFI text mode."));
> > +}
> > +
> > +GRUB_MOD_FINI(cmp)
> > +{
> > +  grub_unregister_command (cmd);
> > +}

Re: [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution

[Paul Menzel]

Dear Glenn,


Am 12.05.22 um 20:29 schrieb Glenn Washburn:

> On Thu, 12 May 2022 08:10:56 +0200 Paul Menzel wrote:

>> Am 12.05.22 um 05:07 schrieb Glenn Washburn:
>>> This command is meant to behave similarly to the 'mode' command of the EFI
>>> Shell application. One difference is that to set the mode the mode number
>>> is given, not the rows and columns of the desired mode. Also supported are
>>> the arguments "min" and "max", which set the mode to the minimum and
>>> maximum mode respectively as calculated by the columns * rows of that mode.
>>
>> Did you test this also with QEMU and OVMF?
> 
> Yes, although with QEMU with -nographic and the output going through
> serial, though not using GRUB's serial terminal. So its still using the
> efi console. Is there a reason you're asking? Something not working as
> expected?

No, I haven’t tested the patch. I just wanted to know, how the patch was 
tested besides the HP device.

[…]


Kind regards,

Paul

[PATCH 2/2] docs: Add documentation for the efitextmode command

[Glenn Washburn]

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 docs/grub.texi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/docs/grub.texi b/docs/grub.texi
index 3b5522b0a..50ef28edd 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4202,6 +4202,7 @@ you forget a command, you can run the command @command{help}
 * distrust::                    Remove a pubkey from trusted keys
 * drivemap::                    Map a drive to another
 * echo::                        Display a line of text
+* efitextmode::                 Set/Get text output mode resolution
 * eval::                        Evaluate agruments as GRUB commands
 * export::                      Export an environment variable
 * false::                       Do nothing, unsuccessfully
@@ -4638,6 +4639,28 @@ character will print that character.
 @end deffn
 
 
+@node efitextmode
+@subsection efitextmode
+
+@deffn Command efitextmode [min | max | mode_num]
+When used with no arguments displays all available text output modes. The
+set mode determines the columns and rows of the text display when in
+text mode. An asterisk, @samp{*}, will be at the end of the line of the
+currently set mode.
+
+Otherwise the command only takes a single parameter, which can be
+@samp{min}, @samp{max}, or a mode number given by the listing when run
+with no arguments. These arguments set the mode to the minimum, maximum,
+and particular mode respectively.
+
+By default GRUB will start in whatever mode the EFI firmware defaults to.
+There are firmwares known to setup the default mode such that output
+behaves strangely. Setting the mode can fix this.
+
+Note: This command is only available on EFI platforms.
+@end deffn
+
+
 @node eval
 @subsection eval
 
-- 
2.34.1

Re: [PATCH 2/2] docs: Add documentation for the efitextmode command

[Paul Menzel]

Dear Glenn,


Thank you for the patch. Two small nits.

Am 12.05.22 um 05:07 schrieb Glenn Washburn:
> Signed-off-by: Glenn Washburn <development@efficientek.com>

*Add documentation for …* could be abbreviated to *Document …* in the 
git commit message summary.

> ---
>   docs/grub.texi | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 3b5522b0a..50ef28edd 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4202,6 +4202,7 @@ you forget a command, you can run the command @command{help}
>  * distrust::                    Remove a pubkey from trusted keys
>  * drivemap::                    Map a drive to another
>  * echo::                        Display a line of text
> +* efitextmode::                 Set/Get text output mode resolution
>  * eval::                        Evaluate agruments as GRUB commands
>  * export::                      Export an environment variable
>  * false::                       Do nothing, unsuccessfully
> @@ -4638,6 +4639,28 @@ character will print that character.
>   @end deffn
>   
>   
> +@node efitextmode
> +@subsection efitextmode
> +
> +@deffn Command efitextmode [min | max | mode_num]
> +When used with no arguments displays all available text output modes. The
> +set mode determines the columns and rows of the text display when in
> +text mode. An asterisk, @samp{*}, will be at the end of the line of the
> +currently set mode.
> +
> +Otherwise the command only takes a single parameter, which can be
> +@samp{min}, @samp{max}, or a mode number given by the listing when run
> +with no arguments. These arguments set the mode to the minimum, maximum,
> +and particular mode respectively.
> +
> +By default GRUB will start in whatever mode the EFI firmware defaults to.
> +There are firmwares known to setup the default mode such that output

The verb *set up* is spelled with a space.

> +behaves strangely. Setting the mode can fix this.

Should your device be mentioned in the documentation, and one of the 
strangeness be described?

> +
> +Note: This command is only available on EFI platforms.
> +@end deffn
> +
> +
>   @node eval
>   @subsection eval


Kind regards,

Paul

Re: [PATCH 2/2] docs: Add documentation for the efitextmode command

[Glenn Washburn]

On Thu, 12 May 2022 08:08:32 +0200
Paul Menzel <pmenzel@molgen.mpg.de> wrote:

> Dear Glenn,
> 
> 
> Thank you for the patch. Two small nits.
> 
> Am 12.05.22 um 05:07 schrieb Glenn Washburn:
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> 
> *Add documentation for …* could be abbreviated to *Document …* in the 
> git commit message summary.
> 
> > ---
> >   docs/grub.texi | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> > 
> > diff --git a/docs/grub.texi b/docs/grub.texi
> > index 3b5522b0a..50ef28edd 100644
> > --- a/docs/grub.texi
> > +++ b/docs/grub.texi
> > @@ -4202,6 +4202,7 @@ you forget a command, you can run the command @command{help}
> >  * distrust::                    Remove a pubkey from trusted keys
> >  * drivemap::                    Map a drive to another
> >  * echo::                        Display a line of text
> > +* efitextmode::                 Set/Get text output mode resolution
> >  * eval::                        Evaluate agruments as GRUB commands
> >  * export::                      Export an environment variable
> >  * false::                       Do nothing, unsuccessfully
> > @@ -4638,6 +4639,28 @@ character will print that character.
> >   @end deffn
> >   
> >   
> > +@node efitextmode
> > +@subsection efitextmode
> > +
> > +@deffn Command efitextmode [min | max | mode_num]
> > +When used with no arguments displays all available text output modes. The
> > +set mode determines the columns and rows of the text display when in
> > +text mode. An asterisk, @samp{*}, will be at the end of the line of the
> > +currently set mode.
> > +
> > +Otherwise the command only takes a single parameter, which can be
> > +@samp{min}, @samp{max}, or a mode number given by the listing when run
> > +with no arguments. These arguments set the mode to the minimum, maximum,
> > +and particular mode respectively.
> > +
> > +By default GRUB will start in whatever mode the EFI firmware defaults to.
> > +There are firmwares known to setup the default mode such that output
> 
> The verb *set up* is spelled with a space.
> 
> > +behaves strangely. Setting the mode can fix this.
> 
> Should your device be mentioned in the documentation, and one of the 
> strangeness be described?

I deliberately was vague here because I don't think the specifics
matter. The point I wanted to get across is that if your text mode
EFI console is behaving strangely (define it how ever you want), that a
mode switch may fix things. This is not to presume that other
strangeness can't be fixed by doing a mode switch. Perhaps I could add
after "strangely", ", for example your cursor in the grub shell never
reaches the bottom of the screen or when typing input characters from
previous command output are overwritten". I don't think my device need
be mentioned.

Glenn

> > +
> > +Note: This command is only available on EFI platforms.
> > +@end deffn
> > +
> > +
> >   @node eval
> >   @subsection eval
> 
> 
> Kind regards,
> 
> Paul

Re: [PATCH 2/2] docs: Add documentation for the efitextmode command

[Paul Menzel]

Dear Glenn,


Am 13.05.22 um 00:46 schrieb Glenn Washburn:
> On Thu, 12 May 2022 08:08:32 +0200 Paul Menzel wrote:

[…]

>> Am 12.05.22 um 05:07 schrieb Glenn Washburn:
>>> Signed-off-by: Glenn Washburn <development@efficientek.com>
>>
>> *Add documentation for …* could be abbreviated to *Document …* in the
>> git commit message summary.
>>
>>> ---
>>>    docs/grub.texi | 23 +++++++++++++++++++++++
>>>    1 file changed, 23 insertions(+)
>>>
>>> diff --git a/docs/grub.texi b/docs/grub.texi
>>> index 3b5522b0a..50ef28edd 100644
>>> --- a/docs/grub.texi
>>> +++ b/docs/grub.texi
>>> @@ -4202,6 +4202,7 @@ you forget a command, you can run the command @command{help}
>>>   * distrust::                    Remove a pubkey from trusted keys
>>>   * drivemap::                    Map a drive to another
>>>   * echo::                        Display a line of text
>>> +* efitextmode::                 Set/Get text output mode resolution
>>>   * eval::                        Evaluate agruments as GRUB commands
>>>   * export::                      Export an environment variable
>>>   * false::                       Do nothing, unsuccessfully
>>> @@ -4638,6 +4639,28 @@ character will print that character.
>>>    @end deffn
>>>    
>>>    
>>> +@node efitextmode
>>> +@subsection efitextmode
>>> +
>>> +@deffn Command efitextmode [min | max | mode_num]
>>> +When used with no arguments displays all available text output modes. The
>>> +set mode determines the columns and rows of the text display when in
>>> +text mode. An asterisk, @samp{*}, will be at the end of the line of the
>>> +currently set mode.
>>> +
>>> +Otherwise the command only takes a single parameter, which can be
>>> +@samp{min}, @samp{max}, or a mode number given by the listing when run
>>> +with no arguments. These arguments set the mode to the minimum, maximum,
>>> +and particular mode respectively.
>>> +
>>> +By default GRUB will start in whatever mode the EFI firmware defaults to.
>>> +There are firmwares known to setup the default mode such that output
>>
>> The verb *set up* is spelled with a space.
>>
>>> +behaves strangely. Setting the mode can fix this.
>>
>> Should your device be mentioned in the documentation, and one of the
>> strangeness be described?
> 
> I deliberately was vague here because I don't think the specifics
> matter. The point I wanted to get across is that if your text mode
> EFI console is behaving strangely (define it how ever you want), that a
> mode switch may fix things. This is not to presume that other
> strangeness can't be fixed by doing a mode switch. Perhaps I could add
> after "strangely", ", for example your cursor in the grub shell never
> reaches the bottom of the screen or when typing input characters from
> previous command output are overwritten". I don't think my device need
> be mentioned.

Yes, I like your suggestion.

[…]


Kind regards,

Paul

Re: [PATCH 0/2] Add efitextmode command

[Gerd Hoffmann]

On Wed, May 11, 2022 at 10:07:33PM -0500, Glenn Washburn wrote:
> This patch add the efitextmode command which is used for showing all
> available text output modes and setting a specific mode. Its basically the
> equivalent of the EFI Shell's "mode" command and its output looks similar.
> The main difference is that its shows the mode number in the listing and
> takes a mode number when setting the mode. As a convenience it can take
> arguments "min" or "max" to set to the minimum or maximum mode.
> 
> Glenn
> 
> Glenn Washburn (2):
>   efi: Add efitextmode command for getting/setting the text mode
>     resolution
>   docs: Add documentation for the efitextmode command

Looks sane to me, works fine on OVMF.

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Tested-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd