[U-Boot] [PATCH] efi: console: Correctly report modes

[Emmanuel Vadot]

On Mon, 19 Sep 2016 15:55:49 +0200
Alexander Graf <agraf at suse.de> wrote:

> 
> 
> On 19.08.16 18:12, Emmanuel Vadot wrote:
> > Hello Alexander,
> > 
> > The UEFI spec say that mode 0 will always be 80x25, mode 1 80x50 and other
> > size will be mode >=2.
> > This patch :
> > - set the default size to 80x25.
> > - returns EFI_UNSUPPORTED if the queried mode isn't available.
> 
> Very well spot, would you mind to tell me where you hit this? :)

 The FreeBSD loader queries mode until it gets EFI_UNSUPPORTED.

> > Signed-off-by: Emmanuel Vadot <manu at bidouilliste.com>
> > ---
> >  lib/efi_loader/efi_console.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> > index 2e0228c..10849f9 100644
> > --- a/lib/efi_loader/efi_console.c
> > +++ b/lib/efi_loader/efi_console.c
> > @@ -9,9 +9,9 @@
> >  #include <common.h>
> >  #include <efi_loader.h>
> >  
> > -/* If we can't determine the console size, default to 80x24 */
> > +/* If we can't determine the console size, default to 80x25 */
> >  static int console_columns = 80;
> > -static int console_rows = 24;
> > +static int console_rows = 25;
> 
> We should probably adapt the comment saying that "mode 0 is always 80x25".
> 
> >  static bool console_size_queried;
> >  
> >  const efi_guid_t efi_guid_console_control = CONSOLE_CONTROL_GUID;
> > @@ -165,6 +165,8 @@ static efi_status_t EFIAPI efi_cout_query_mode(
> >  			unsigned long mode_number, unsigned long *columns,
> >  			unsigned long *rows)
> >  {
> > +	unsigned long current_mode;
> > +
> >  	EFI_ENTRY("%p, %ld, %p, %p", this, mode_number, columns, rows);
> >  
> >  	if (!console_size_queried) {
> > @@ -196,6 +198,16 @@ static efi_status_t EFIAPI efi_cout_query_mode(
> >  	}
> >  
> >  out:
> > +	if (console_columns == 80 && console_rows == 25)
> > +		current_mode = 0;
> > +	else if (console_columns == 80 && console_rows == 50)
> > +		current_mode = 1;
> > +	else
> > +		current_mode = 2;
> 
> Unfortunately this introduces modes that we don't declare as supported.
> The struct "efi_con_mode" only sets max_mode to 0. So we really tell the
> payload that we only have 80x25 available.
> 
> I guess we should set max_mode to 2 and have mode 2 be the actually
> determined size while mode 0 stays 80x25? This will need some
> refactoring of the code.

 Ah true, I've missed that, I'll update my patch this week.

> Sorry for the terribly late reply.
> 
> Alex

 No problem,

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