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

Rob Clark robdclark at gmail.com
Thu Oct 5 00:12:46 UTC 2017


On Wed, Oct 4, 2017 at 8:00 PM, Rob Clark <robdclark at gmail.com> wrote:
> On Wed, Oct 4, 2017 at 7:53 PM, Heinrich Schuchardt <xypron.glpk at 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 at 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 at 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 at 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 ;-)

hmm, the semi-annoying thing will be to have to implement the
vidconsole-uclass side of this.. I suppose I could ignore anything
other than 0 (normal) and 1 (bold).  Reverse wouldn't be too hard, but
blink would be hard I think without timer interrupts (same reason that
I didn't implement cursor yet)

> 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
>>>>
>>>
>>


More information about the U-Boot mailing list