Re: [edk2-devel] [edk2-platform][PATCH v2 3/3] MdeModulePkg/ConSplitter: Change StdErr color to EFI_LIGHTGRAY

["Gao, Zhichao" <zhichao.gao@intel.com>]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Pete Batard <pete@akeo.ie>
> Sent: Thursday, December 24, 2020 12:35 AM
> To: devel@edk2.groups.io; samer.el-haj-mahmoud@arm.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard
> Biesheuvel <Ard.Biesheuvel@arm.com>
> Subject: Re: [edk2-devel] [edk2-platform][PATCH v2 3/3]
> MdeModulePkg/ConSplitter: Change StdErr color to EFI_LIGHTGRAY
> 
> On 2020.12.20 13:53, Samer El-Haj-Mahmoud wrote:
> > ConSplitter is using EFI_LIGHTGRAY foreground color for ConOut and
> > EFI_MAGENTA for StdErr consoles. This is impacting the DebugLib output
> > on that same serial console (e.g. DebugLibSerialPort) after
> > gEfiStandardErrorDeviceGuid is installed on that port. The impact also
> > extends to Linux serial console output in OVMF because it inherits the
> > color setting from the firmware.
> >
> > This is inconsistent and annoying, with MAGENTA being barely legible
> > on a black background.
> >
> > Let's change StdErr default color to LIGHTGRAY for consistency and
> > readability. This results in the same color being used for all
> > consoles sharing the same serial port (ConOut, StdErr, DebugLib, OS console).
> > Platforms wishing to distinguish the colors of consoles can do so in
> > their own Platform BDS initialization.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
> > Signed-off-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> > ---
> >   MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> > a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> > b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> > index b090de288517..e8cd4ce120a0 100644
> > --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> > +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> > @@ -1476,7 +1476,7 @@ ConSplitterStdErrDriverBindingStart (
> >     // their MaxMode and QueryData should be the intersection of both.
> >
> >     //
> >
> >     Status = ConSplitterTextOutAddDevice (&mStdErr, TextOut, NULL,
> > NULL);
> >
> > -  ConSplitterTextOutSetAttribute (&mStdErr.TextOut, EFI_TEXT_ATTR
> > (EFI_MAGENTA, EFI_BLACK));
> >
> > +  ConSplitterTextOutSetAttribute (&mStdErr.TextOut, EFI_TEXT_ATTR
> > + (EFI_LIGHTGRAY, EFI_BLACK));
> >
> >
> >
> >     return Status;
> >
> >   }
> >
> 
> Reviewed-by: Pete Batard <pete@akeo.ie>
> Tested-by: Pete Batard <pete@akeo.ie> (On an RPi 4 platform where this was
> another annoyance)