Re: [PATCH v3 2/3] efi_loader: Set default console colors on efi_cout_clear_screen if needed

[Jan Kiszka <jan.kiszka@siemens.com>]

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&section=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

-- 
Siemens AG, Technology
Competence Center Embedded Linux