All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add efitextmode command
@ 2022-05-12  3:07 Glenn Washburn
  2022-05-12  3:07 ` [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution Glenn Washburn
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Glenn Washburn @ 2022-05-12  3:07 UTC (permalink / raw)
  To: grub-devel, Daniel Kiper; +Cc: Gerd Hoffmann, 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



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

* [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution
  2022-05-12  3:07 [PATCH 0/2] Add efitextmode command Glenn Washburn
@ 2022-05-12  3:07 ` Glenn Washburn
  2022-05-12  6:10   ` Paul Menzel
  2022-05-12  3:07 ` [PATCH 2/2] docs: Add documentation for the efitextmode command Glenn Washburn
  2022-05-13  5:07 ` [PATCH 0/2] Add " Gerd Hoffmann
  2 siblings, 1 reply; 10+ messages in thread
From: Glenn Washburn @ 2022-05-12  3:07 UTC (permalink / raw)
  To: grub-devel, Daniel Kiper; +Cc: Gerd Hoffmann, 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



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

* [PATCH 2/2] docs: Add documentation for the efitextmode command
  2022-05-12  3:07 [PATCH 0/2] Add efitextmode command Glenn Washburn
  2022-05-12  3:07 ` [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution Glenn Washburn
@ 2022-05-12  3:07 ` Glenn Washburn
  2022-05-12  6:08   ` Paul Menzel
  2022-05-13  5:07 ` [PATCH 0/2] Add " Gerd Hoffmann
  2 siblings, 1 reply; 10+ messages in thread
From: Glenn Washburn @ 2022-05-12  3:07 UTC (permalink / raw)
  To: grub-devel, Daniel Kiper; +Cc: Gerd Hoffmann, 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



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

* Re: [PATCH 2/2] docs: Add documentation for the efitextmode command
  2022-05-12  3:07 ` [PATCH 2/2] docs: Add documentation for the efitextmode command Glenn Washburn
@ 2022-05-12  6:08   ` Paul Menzel
  2022-05-12 22:46     ` Glenn Washburn
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Menzel @ 2022-05-12  6:08 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: The development of GNU GRUB, Gerd Hoffmann, Daniel Kiper

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


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

* Re: [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution
  2022-05-12  3:07 ` [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution Glenn Washburn
@ 2022-05-12  6:10   ` Paul Menzel
  2022-05-12 18:29     ` Glenn Washburn
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Menzel @ 2022-05-12  6:10 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: The development of GNU GRUB, Gerd Hoffmann, Daniel Kiper

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);
> +}


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

* Re: [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution
  2022-05-12  6:10   ` Paul Menzel
@ 2022-05-12 18:29     ` Glenn Washburn
  2022-05-12 20:37       ` Paul Menzel
  0 siblings, 1 reply; 10+ messages in thread
From: Glenn Washburn @ 2022-05-12 18:29 UTC (permalink / raw)
  To: Paul Menzel; +Cc: The development of GNU GRUB, Gerd Hoffmann, Daniel Kiper

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);
> > +}


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

* Re: [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution
  2022-05-12 18:29     ` Glenn Washburn
@ 2022-05-12 20:37       ` Paul Menzel
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Menzel @ 2022-05-12 20:37 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: The development of GNU GRUB, Gerd Hoffmann, Daniel Kiper

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


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

* Re: [PATCH 2/2] docs: Add documentation for the efitextmode command
  2022-05-12  6:08   ` Paul Menzel
@ 2022-05-12 22:46     ` Glenn Washburn
  2022-05-13  6:41       ` Paul Menzel
  0 siblings, 1 reply; 10+ messages in thread
From: Glenn Washburn @ 2022-05-12 22:46 UTC (permalink / raw)
  To: Paul Menzel; +Cc: The development of GNU GRUB, Gerd Hoffmann, Daniel Kiper

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


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

* Re: [PATCH 0/2] Add efitextmode command
  2022-05-12  3:07 [PATCH 0/2] Add efitextmode command Glenn Washburn
  2022-05-12  3:07 ` [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution Glenn Washburn
  2022-05-12  3:07 ` [PATCH 2/2] docs: Add documentation for the efitextmode command Glenn Washburn
@ 2022-05-13  5:07 ` Gerd Hoffmann
  2 siblings, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2022-05-13  5:07 UTC (permalink / raw)
  To: Glenn Washburn; +Cc: grub-devel, Daniel Kiper

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



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

* Re: [PATCH 2/2] docs: Add documentation for the efitextmode command
  2022-05-12 22:46     ` Glenn Washburn
@ 2022-05-13  6:41       ` Paul Menzel
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Menzel @ 2022-05-13  6:41 UTC (permalink / raw)
  To: grub-devel

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


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

end of thread, other threads:[~2022-05-13  6:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12  3:07 [PATCH 0/2] Add efitextmode command Glenn Washburn
2022-05-12  3:07 ` [PATCH 1/2] efi: Add efitextmode command for getting/setting the text mode resolution Glenn Washburn
2022-05-12  6:10   ` Paul Menzel
2022-05-12 18:29     ` Glenn Washburn
2022-05-12 20:37       ` Paul Menzel
2022-05-12  3:07 ` [PATCH 2/2] docs: Add documentation for the efitextmode command Glenn Washburn
2022-05-12  6:08   ` Paul Menzel
2022-05-12 22:46     ` Glenn Washburn
2022-05-13  6:41       ` Paul Menzel
2022-05-13  5:07 ` [PATCH 0/2] Add " Gerd Hoffmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.