[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

Rob Clark
2017-10-05T00:00:14Z

On Wed, Oct 4, 2017 at 7:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 10/05/2017 01:19 AM, Rob Clark wrote:
>> On Wed, Oct 4, 2017 at 6:01 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 10/04/2017 10:54 PM, Rob Clark wrote:
>>>> On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>>>> Shell.efi uses this, and supporting color attributes makes things look
>>>>>> nicer. Map the EFI fg/bg color attributes to ANSI escape sequences.
>>>>>> Not all colors have a perfect match, but spec just says "Devices
>>>>>> supporting a different number of text colors are required to emulate the
>>>>>> above colors to the best of the device’s capabilities".
>>>>>>
>>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>>> ---
>>>>>> include/efi_api.h | 29 +++++++++++++++++++++++++++++
>>>>>> lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 59 insertions(+)
>>>>>>
>>>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>>>> index 87c8ffe68e..3cc1dbac2e 100644
>>>>>> --- a/include/efi_api.h
>>>>>> +++ b/include/efi_api.h
>>>>>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>>>>> EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>>>>> 0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>>>>>
>>>>>> +#define EFI_BLACK 0x00
>>>>>> +#define EFI_BLUE 0x01
>>>>>> +#define EFI_GREEN 0x02
>>>>>> +#define EFI_CYAN 0x03
>>>>>> +#define EFI_RED 0x04
>>>>>> +#define EFI_MAGENTA 0x05
>>>>>> +#define EFI_BROWN 0x06
>>>>>> +#define EFI_LIGHTGRAY 0x07
>>>>>> +#define EFI_BRIGHT 0x08
>>>>>> +#define EFI_DARKGRAY 0x08
>>>>>> +#define EFI_LIGHTBLUE 0x09
>>>>>> +#define EFI_LIGHTGREEN 0x0a
>>>>>> +#define EFI_LIGHTCYAN 0x0b
>>>>>> +#define EFI_LIGHTRED 0x0c
>>>>>> +#define EFI_LIGHTMAGENTA 0x0d
>>>>>> +#define EFI_YELLOW 0x0e
>>>>>> +#define EFI_WHITE 0x0f
>>>>>> +#define EFI_BACKGROUND_BLACK 0x00
>>>>>> +#define EFI_BACKGROUND_BLUE 0x10
>>>>>> +#define EFI_BACKGROUND_GREEN 0x20
>>>>>> +#define EFI_BACKGROUND_CYAN 0x30
>>>>>> +#define EFI_BACKGROUND_RED 0x40
>>>>>> +#define EFI_BACKGROUND_MAGENTA 0x50
>>>>>> +#define EFI_BACKGROUND_BROWN 0x60
>>>>>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>>>>>
>>>>> Will we ever use these constants?
>>>>>
>>>>
>>>> possibly not, but it is useful to understand what is going on with
>>>> efi->ansi mapping, so I would prefer to keep them.
>>>>
>>>>>
>>>>> Where are the comments explaining the defines below?
>>>>>
>>>>>> +
>>>>>> +#define EFI_ATTR_FG(attr) ((attr) & 0x0f)
>>>>>
>>>>> This saves 8 entries in the table below.
>>>>> +#define EFI_ATTR_FG(attr) ((attr) & 0x07)
>>>>>
>>>>>> +#define EFI_ATTR_BG(attr) (((attr) >> 4) & 0x7)
>>>>>
>>>>> Add
>>>>> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>>>>>
>>>>>> +
>>>>>> struct efi_simple_text_output_protocol {
>>>>>> void *reset;
>>>>>> efi_status_t (EFIAPI *output_string)(
>>>>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>>>>> index 2e13fdc096..fcd65ca488 100644
>>>>>> --- a/lib/efi_loader/efi_console.c
>>>>>> +++ b/lib/efi_loader/efi_console.c
>>>>>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>>>>> return EFI_EXIT(EFI_SUCCESS);
>>>>>> }
>>>>>>
>>>>>> +static const struct {
>>>>>> + unsigned fg;
>>>>>> + unsigned bg;
>>>>>> +} color[] = {
>>>>>> + { 30, 40 }, /* 0: black */
>>>>>> + { 34, 44 }, /* 1: blue */
>>>>>> + { 32, 42 }, /* 2: green */
>>>>>> + { 36, 46 }, /* 3: cyan */
>>>>>> + { 31, 41 }, /* 4: red */
>>>>>> + { 35, 45 }, /* 5: magenta */
>>>>>> + { 30, 40 }, /* 6: brown, map to black */
>>>>>
>>>>> This should be { 33, 43 }
>>>>>
>>>>>> + { 37, 47 }, /* 7: light grey, map to white */
>>>>>
>>>>> The entries below are redundant.
>>>>>
>>>>>> + { 37, 47 }, /* 8: bright, map to white */
>>>>>> + { 34, 44 }, /* 9: light blue, map to blue */
>>>>>> + { 32, 42 }, /* A: light green, map to green */
>>>>>> + { 36, 46 }, /* B: light cyan, map to cyan */
>>>>>> + { 31, 41 }, /* C: light red, map to red */
>>>>>> + { 35, 45 }, /* D: light magenta, map to magenta */
>>>>>> + { 33, 43 }, /* E: yellow */
>>>>>> + { 37, 47 }, /* F: white */
>>>>>> +};
>>>>>> +
>>>>
>>>> I'm not totally convinced about mapping extra colors that UEFI defines
>>>> to bold.. unless you have some example of prior-art for this on other
>>>> platforms.
>>>
>>> See
>>> Standard ECMA-48 - Control Functions for Coded Character Sets
>>> chapter 8.3.117 SGR - SELECT GRAPHIC RENDITION
>>>
>>> 1 - bold or increased intensity
>>> 22 - normal colour or normal intensity (neither bold nor faint)
>>>
>>> You can easily experiment in your bash shell like this:
>>>
>>> printf "\x1b[1;32;40m bold \x1b[22;32;40m normal\x1b[22;39;49m\n";
>>>
>>> You will find that "bold" prints bold and bright in the KDE konsole and
>>> xterm.
>>
>> but I think we don't want (potential) font changes, just color changes..
>>
>> if you can find the code in edk2 that does this, I guess it would be a
>> reasonable precedent to follow.. but if not I wanted to avoid things
>> that might be specific to particular terminal emulators, since I
>> wasn't really looking forward to testing them all. Otherwise I'd just
>> rely on the extension that allowed 256 colors..
>>
>> BR,
>> -R
>
> The same problem seems has led the EDK folks to a similar solution.
>
> See
> MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c

ok, I'll have a closer look at that.. I don't feel badly about doing
the same thing that edk2 does when there is doubt ;-)

BR,
-R

> Everything starts with this array:
>
> { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm', ESC, '[', '4', '0', 'm', 0 };
>
> The first '0' is replaced by either 0 or 1 depending on brightness.
>
> mSetAttributeString[BRIGHT_CONTROL_OFFSET] =
> (CHAR16) ('0' + BrightControl);
>
> The first '4', '0' is replaced by the foreground color.
> The second '4', '0' is replaced by the background color.
>
> ECMA 48 says:
>
> 0 - default rendition, cancels the effect of any preceding SGR
>
> So you can use this instead of 22.
>
> Best regards
>
> Heinrich
>
>
>>
>>> Using colors 90-97 as foreground colors produces only bright but not
>>> bold in the KDE konsole and xterm:
>>>
>>> printf "\x1b[92;40m bold \x1b[32;40m normal\x1b[22;39;49m\n";
>>>
>>> But these codes are not defined in ECMA-48.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>
>