| Message ID | 4b7a28d04119ae529a2dc1a16dec50fb39c74f17.1667549199.git.jan.kiszka@siemens.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: console and network dependency improvements | expand |
On 04.11.22 09:06, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Ensures a consistent background color of the whole screen for succeeding > outputs as both demanded by the spec and implemented in EDK2 as well. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > lib/efi_loader/efi_console.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c > index 6ce0fcc168d..4228a509caf 100644 > --- a/lib/efi_loader/efi_console.c > +++ b/lib/efi_loader/efi_console.c > @@ -495,6 +495,12 @@ static efi_status_t EFIAPI efi_cout_clear_screen( > { > EFI_ENTRY("%p", this); > > + /* Set default colors if not done yet */ > + if (efi_con_mode.attribute == 0) { > + efi_con_mode.attribute = 0x07; > + printf(ESC "[0;37;40m"); > + } > + > efi_clear_screen(); > > return EFI_EXIT(EFI_SUCCESS); And what about this visual fix? It does not depend on patch 1. Jan
On 11/4/22 09:06, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Ensures a consistent background color of the whole screen for succeeding > outputs as both demanded by the spec and implemented in EDK2 as well. To which sentence in the UEFI 2.10 spec are you relating? > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > lib/efi_loader/efi_console.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c > index 6ce0fcc168d..4228a509caf 100644 > --- a/lib/efi_loader/efi_console.c > +++ b/lib/efi_loader/efi_console.c > @@ -495,6 +495,12 @@ static efi_status_t EFIAPI efi_cout_clear_screen( > { > EFI_ENTRY("%p", this); > > + /* Set default colors if not done yet */ > + if (efi_con_mode.attribute == 0) { > + efi_con_mode.attribute = 0x07; > + printf(ESC "[0;37;40m"); These values are correct for CONFIG_SYS_WHITE_ON_BLACK=y. But for CONFIG_SYS_WHITE_ON_BLACK=n I would expect black letters on white as default. This requires adding support background color values 100 - 107 (cf. https://en.wikipedia.org/w/index.php?title=ANSI_escape_code§ion=15#3-bit_and_4-bit) in vidconsole_escape_char(). UEFI background color white should be translated to 107 if CONFIG_SYS_WHITE_ON_BLACK=n. It seems this patch is trying to solve a problem caused by patch 1 of the series. Or do I miss something? Best regards Heinrich > + } > + > efi_clear_screen(); > > return EFI_EXIT(EFI_SUCCESS);
On 07.11.22 18:06, Heinrich Schuchardt wrote: > On 11/4/22 09:06, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Ensures a consistent background color of the whole screen for succeeding >> outputs as both demanded by the spec and implemented in EDK2 as well. > > To which sentence in the UEFI 2.10 spec are you relating? > 12.4: "EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen() Clears the output device(s) display to the currently selected background color." That generally works - except for this default case. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> lib/efi_loader/efi_console.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c >> index 6ce0fcc168d..4228a509caf 100644 >> --- a/lib/efi_loader/efi_console.c >> +++ b/lib/efi_loader/efi_console.c >> @@ -495,6 +495,12 @@ static efi_status_t EFIAPI efi_cout_clear_screen( >> { >> EFI_ENTRY("%p", this); >> >> + /* Set default colors if not done yet */ >> + if (efi_con_mode.attribute == 0) { >> + efi_con_mode.attribute = 0x07; >> + printf(ESC "[0;37;40m"); > > These values are correct for CONFIG_SYS_WHITE_ON_BLACK=y. But for > CONFIG_SYS_WHITE_ON_BLACK=n I would expect black letters on white as > default. > > This requires adding support background color values 100 - 107 (cf. > https://en.wikipedia.org/w/index.php?title=ANSI_escape_code§ion=15#3-bit_and_4-bit) > in vidconsole_escape_char(). > > UEFI background color white should be translated to 107 if > CONFIG_SYS_WHITE_ON_BLACK=n. I don't think we handle CONFIG_SYS_WHITE_ON_BLACK=n in UEFI so far. It likely requires translating all colors that apps so far select. Otherwise, you will end up with white-on-EFI_WHITE. > > It seems this patch is trying to solve a problem caused by patch 1 of > the series. Or do I miss something? Not at all, problem persists if you drop that patch. I've accidentally accounted to "efi_loader: avoid EFI_CALL() for clearing screen", see https://lore.kernel.org/u-boot/532ff3e2-06ba-3a3e-5cf9-2f4a9b7abb8c@siemens.com/ for a screen shot. Jan
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 6ce0fcc168d..4228a509caf 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -495,6 +495,12 @@ static efi_status_t EFIAPI efi_cout_clear_screen( { EFI_ENTRY("%p", this); + /* Set default colors if not done yet */ + if (efi_con_mode.attribute == 0) { + efi_con_mode.attribute = 0x07; + printf(ESC "[0;37;40m"); + } + efi_clear_screen(); return EFI_EXIT(EFI_SUCCESS);