On 2016-05-25 14:13:03, Laszlo Ersek wrote: > On 05/25/16 21:53, Jordan Justen wrote: > > On 2016-05-25 10:23:56, Laszlo Ersek wrote: > >> On 05/25/16 19:00, Jordan Justen wrote: > >>> On 2016-05-25 05:36:43, Laszlo Ersek wrote: > >>>> OVMF's Platform BDS used to have a nice progress bar (with > >>>> IntelFrameworkModulePkg BDS). We can restore it by copying the > >>>> PlatformBootManagerWaitCallback() function verbatim from > >>>> > >>>> Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c > >>>> > >>>> It can be tested by passing the following option to QEMU (5 seconds): > >>>> > >>>> -boot menu=on,splash-time=5000 > >>>> > >>>> Cc: Jordan Justen <[email protected]> > >>>> Cc: Ruiyu Ni <[email protected]> > >>>> Contributed-under: TianoCore Contribution Agreement 1.0 > >>>> Signed-off-by: Laszlo Ersek <[email protected]> > >>>> --- > >>>> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 17 > >>>> +++++++++++++++++ > >>>> 1 file changed, 17 insertions(+) > >>>> > >>>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > >>>> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > >>>> index 9eb9e390373d..dd8757f58ec3 100644 > >>>> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > >>>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > >>>> @@ -1425,5 +1425,22 @@ PlatformBootManagerWaitCallback ( > >>>> UINT16 TimeoutRemain > >>>> ) > >>>> { > >>>> + EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black; > >>>> + EFI_GRAPHICS_OUTPUT_BLT_PIXEL White; > >>>> + UINT16 Timeout; > >>>> + > >>>> + Timeout = PcdGet16 (PcdPlatformBootTimeOut); > >>>> + > >>>> + Black.Blue = Black.Green = Black.Red = Black.Reserved = 0; > >>>> + White.Blue = White.Green = White.Red = White.Reserved = 0xFF; > >>> > >>> I know this came from Nt32, but how about making these global vars? > >>> > >>> static EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black = { 0, 0, 0, 0 }; > >>> static EFI_GRAPHICS_OUTPUT_BLT_PIXEL White = { 0xff, 0xff, 0xff, 0 }; > >> > >> I'm okay with giving them static storage duration. > >> > >> With that changed, > >> - do you want me to keep them in function scope (with the same names), > >> - or should I move them to file scope (and then, as Andrew suggests, > >> rename them to, say, mPixelBlack and mPixelWhite)? > >> > > > > The part that bugged me was setting .Reserved to 0xff. > > > > How about just making them initialized local variables? > > > > EFI_GRAPHICS_OUTPUT_BLT_PIXEL Black = { 0, 0, 0, 0 }; > > EFI_GRAPHICS_OUTPUT_BLT_PIXEL White = { 0xff, 0xff, 0xff, 0 }; > > That would conflict with the edk2 coding style (6.8 C Function Layout, > "Initializing a variable as part of its declaration is illegal" -- > although it clearly doesn't cover variables with static storage duration.) >
I do see that in the older style guide, but not the newer ones (2.0+). > I even suspect that the first initialization could cause some compilers > to emit memset() intrinsics. > Hmm, yeah that could be a problem. > How about: > > Black.Blue = Black.Green = Black.Red = 0; > White.Blue = White.Green = White.Red = 0xFF; > Black.Reserved = White.Reserved = 0; > Sure, or a static local var. I hope that in the case of the above code that most compilers would see that we are really just dealing with two uint32_t values. I guess there is the union wrapper of the struct, and combined with the UEFI assumption of little-endian, you could force it. -Jordan > (I could submit a patch for Nt32Pkg too that fixed White.Reserved.) > > Thanks > Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
