On 04/23/19 09:04, Zhichao Gao wrote: > From: Aaron Antone <[email protected]> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1412 > > For now, most platform support to display during PEIM. It means the logo > can show at PEI phase. But the screen would be cleared at BDS connect > console phase. That may make the screen flush and turn into black screen. > So do not clear the screen while set the text mode for graphics console > device for the first time boot. > As the shell reconnect command would make the same reslut with the first > boot, use the gEfiEventReadyToBootGuid to distinguish them. > > Also replace the debug code in GraphicsConsoleControllerDriverStart. The > origin one would set a basic mode and then print the text info to graphic > console device. Then the conspliter would set a best mode for graphics > console device. If the best mode is different with the basic one, the > screen would be cleared. So use the debug output instead. > > This patch only affect the behavior of SetMode at the first boot during > the graphics console devices first connect operations. That means at > DXE phase before ReadyToBoot, the Graphics Simple Text Out SetMode would not > clear the screen during the first connecttion of the graphics devices.
The UEFI spec requirement doesn't apply after ReadyToBoot only. What about SysPrep applications, for example: """ When launched, the platform is required to provide the application loaded by SysPrep####, with the same services such as console and network as are normally provided at launch to applications referenced by a Boot#### variable. [...] After all SysPrep#### variables have been launched and exited, the platform shall notify EFI_EVENT_GROUP_READY_TO_BOOT event group and begin to evaluate Boot#### variables [...] """ Thus a SysPrep application is permitted to expect, and to use, the console, but it is launched before ReadyToBoot; and so this patch could break the console's std conformance for SysPrep apps. I guess you have already investigated adding a boolean field to GRAPHICS_CONSOLE_DEV, and found it unsuitable for those platforms that need this anti-flicker tweak. So I'm not going to suggest such a boolean field now. Instead, I propose a PCD (feature PCD or dynamic boolean PCD). If you add a PCD, I won't care about the particulars of this patch, as long as platforms continue observing the std conformant behavior, under the default value of the PCD (i.e., from "MdeModulePkg.dec"). Thanks, Laszlo > Cc: Jian J Wang <[email protected]> > Cc: Hao Wu <[email protected]> > Cc: Ray Ni <[email protected]> > Cc: Star Zeng <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Sean Brogan <[email protected]> > Cc: Michael Turner <[email protected]> > Cc: Bret Barkelew <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Signed-off-by: Zhichao Gao <[email protected]> > --- > .../GraphicsConsoleDxe/GraphicsConsole.c | 82 +++++++++++++------ > .../GraphicsConsoleDxe/GraphicsConsoleDxe.inf | 3 + > 2 files changed, 62 insertions(+), 23 deletions(-) > > diff --git > a/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsole.c > b/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsole.c > index 26ea19f300..39a999838c 100644 > --- a/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsole.c > +++ b/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsole.c > @@ -1,7 +1,7 @@ > /** @file > This is the main routine for initializing the Graphics Console support > routines. > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -96,6 +96,12 @@ EFI_DRIVER_BINDING_PROTOCOL gGraphicsConsoleDriverBinding > = { > NULL > }; > > +// > +// Event and variable to indicate the boot phase. > +// > +EFI_EVENT mGraphicsConsoleReadyToBootEvent; > +BOOLEAN mGraphicsConsoleReadyToBoot = FALSE; > + > /** > Test to see if Graphics Console could be supported on the Controller. > > @@ -567,16 +573,7 @@ GraphicsConsoleControllerDriverStart ( > // > Private->SimpleTextOutputMode.MaxMode = (INT32) MaxMode; > > - DEBUG_CODE_BEGIN (); > - Status = GraphicsConsoleConOutSetMode (&Private->SimpleTextOutput, 0); > - if (EFI_ERROR (Status)) { > - goto Error; > - } > - Status = GraphicsConsoleConOutOutputString (&Private->SimpleTextOutput, > (CHAR16 *)L"Graphics Console Started\n\r"); > - if (EFI_ERROR (Status)) { > - goto Error; > - } > - DEBUG_CODE_END (); > + DEBUG ((DEBUG_INFO, "Graphics Console Started!\n\r")); > > // > // Install protocol interfaces for the Graphics Console device. > @@ -1366,18 +1363,26 @@ GraphicsConsoleConOutSetMode ( > // > // The current graphics mode is correct, so simply clear the entire > display > // > - Status = GraphicsOutput->Blt ( > - GraphicsOutput, > - &mGraphicsEfiColors[0], > - EfiBltVideoFill, > - 0, > - 0, > - 0, > - 0, > - ModeData->GopWidth, > - ModeData->GopHeight, > - 0 > - ); > + // > + // For the first time boot system, do not clear the display. > + // Some platforms would show logo at PEIM and this would clear > + // the whole screen. So for first boot set mode, do not clear > + // the screen. > + // > + if (This->Mode->Mode != -1 || mGraphicsConsoleReadyToBoot) { > + Status = GraphicsOutput->Blt ( > + GraphicsOutput, > + &mGraphicsEfiColors[0], > + EfiBltVideoFill, > + 0, > + 0, > + 0, > + 0, > + ModeData->GopWidth, > + ModeData->GopHeight, > + 0 > + ); > + } > } > } else if (FeaturePcdGet (PcdUgaConsumeSupport)) { > // > @@ -2065,6 +2070,24 @@ RegisterFontPackage ( > FreePool (Package); > } > > +/** > + Indicate the Boot phase is at ReadyToBoot. > + > + @param[in] Event The Event that is being processed. > + @param[in] Context The Event Context. > + > +**/ > +VOID > +GraphicsConsoleReadyToBootEvent ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + mGraphicsConsoleReadyToBoot = TRUE; > + > + gBS->CloseEvent (mGraphicsConsoleReadyToBootEvent); > +} > + > /** > The user Entry Point for module GraphicsConsole. The user code starts with > this function. > > @@ -2095,6 +2118,19 @@ InitializeGraphicsConsole ( > &mHiiRegistration > ); > > + // > + // Create a event function of ReadyToBoot to avoid clearing screen before > boot > + // > + Status = gBS->CreateEventEx ( > + EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + GraphicsConsoleReadyToBootEvent, > + NULL, > + &gEfiEventReadyToBootGuid, > + &mGraphicsConsoleReadyToBootEvent > + ); > + ASSERT_EFI_ERROR (Status); > + > // > // Install driver model protocol(s). > // > diff --git > a/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf > b/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf > index f7caa65aa9..59751893f6 100644 > --- a/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf > +++ b/MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf > @@ -49,6 +49,9 @@ > HiiLib > PcdLib > > +[Guids] > + gEfiEventReadyToBootGuid ## CONSUMES > + > [Protocols] > gEfiDevicePathProtocolGuid ## TO_START > gEfiSimpleTextOutProtocolGuid ## BY_START > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39416): https://edk2.groups.io/g/devel/message/39416 Mute This Topic: https://groups.io/mt/31306535/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
