Hi Sean, Yes, that is the correct section. Hard to tell from patch email alone. There is a git config that can always include the name of the section of the INF/DEC/DSC/FDF file where a change is made. Can make it a bit easier to review. https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05 Specifically this one I think: git config diff.ini.xfuncname '^\[[A-Za-z0-9_., ]+]' Mike From: Sean Rhodes Sent: Thursday, December 15, 2022 2:17 PM To: Kinney, Michael D Cc: devel@edk2.groups.io; Gao, Zhichao ; Ni, Ray ; Wang, Jian J ; Gao, Liming Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Hi Mike Thank you; changed to PcdGetBool. It's in `[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]` - is that not right? Thanks Sean On Thu, 15 Dec 2022 at 22:09, Kinney, Michael D > wrote: Hi Sean, A couple comments related to the PCD type below. Mike > -----Original Message----- > From: devel@edk2.groups.io > On Behalf Of Sean Rhodes > Sent: Thursday, December 15, 2022 1:12 PM > To: devel@edk2.groups.io > Cc: Rhodes, Sean >; Gao, Zhichao >; Ni, Ray >; Wang, Jian J > >; Gao, Liming > > Subject: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo > > When set to true, the Logo is positioned according to the BGRT > specification, 38.2% from the top of the screen. When set to false, > no behaviour is changed and the logo is positioned centrally. > > Cc: Zhichao Gao > > Cc: Ray Ni > > Cc: Jian J Wang > > Cc: Liming Gao > > Signed-off-by: Sean Rhodes > > --- > MdeModulePkg/Logo/Logo.c | 28 +++++++++++++++++++++++++++- > MdeModulePkg/Logo/LogoDxe.inf | 4 ++++ > MdeModulePkg/MdeModulePkg.dec | 6 ++++++ > MdeModulePkg/MdeModulePkg.uni | 6 ++++++ > 4 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c > index 8ab874d2da..48862d3207 100644 > --- a/MdeModulePkg/Logo/Logo.c > +++ b/MdeModulePkg/Logo/Logo.c > @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include > > #include > > #include > > +#include > > > > typedef struct { > > EFI_IMAGE_ID ImageId; > > @@ -51,12 +52,14 @@ GetImage ( > IN EDKII_PLATFORM_LOGO_PROTOCOL *This, > > IN OUT UINT32 *Instance, > > OUT EFI_IMAGE_INPUT *Image, > > + EFI_GRAPHICS_OUTPUT_PROTOCOL *GraphicsOutput, > > OUT EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE *Attribute, > > OUT INTN *OffsetX, > > OUT INTN *OffsetY > > ) > > { > > - UINT32 Current; > > + UINT32 Current; > > + EFI_STATUS Status; > > > > if ((Instance == NULL) || (Image == NULL) || > > (Attribute == NULL) || (OffsetX == NULL) || (OffsetY == NULL)) > > @@ -69,6 +72,29 @@ GetImage ( > return EFI_NOT_FOUND; > > } > > > > + if (FixedPcdGetBool (PcdFollowMicrosoftRecommended)) { Should be PcdGetBool(). The only time FixedPcdGetxxx() is required is if the PCD value is being used to initialize a struct where the value is needed at build time. This allows the PCD type to be flexible and can be set in platform scope in the DSC file. > > + // > > + // Get current video resolution and text mode > > + // > > + Status = gBS->HandleProtocol ( > > + gST->ConsoleOutHandle, > > + &gEfiGraphicsOutputProtocolGuid, > > + (VOID **)&GraphicsOutput > > + ); > > + if (!EFI_ERROR (Status)) { > > + // > > + // Center of LOGO is in the vertical position 38.2% when PcdBootLogoOnlyEnable is TRUE > > + // Y = (VerticalResolution - LogoHeight) / 2 > > + // Y' = VerticalResolution * 0.382 - LogoHeight * 0.5 > > + // OffsetY + Y = Y' > > + // OffsetY = Y' - Y = -0.118 * VerticalResolution > > + // > > + *Attribute = EdkiiPlatformLogoDisplayAttributeCenter; > > + *OffsetX = 0; > > + *OffsetY = -118 * (INTN)GraphicsOutput->Mode->Info->VerticalResolution / 1000; > > + } > > + } > > + > > (*Instance)++; > > *Attribute = mLogos[Current].Attribute; > > *OffsetX = mLogos[Current].OffsetX; > > diff --git a/MdeModulePkg/Logo/LogoDxe.inf b/MdeModulePkg/Logo/LogoDxe.inf > index 41215d25d8..ce29950089 100644 > --- a/MdeModulePkg/Logo/LogoDxe.inf > +++ b/MdeModulePkg/Logo/LogoDxe.inf > @@ -41,6 +41,7 @@ > UefiBootServicesTableLib > > UefiDriverEntryPoint > > DebugLib > > + PcdLib > > > > [Protocols] > > gEfiHiiDatabaseProtocolGuid ## CONSUMES > > @@ -48,6 +49,9 @@ > gEfiHiiPackageListProtocolGuid ## PRODUCES CONSUMES > > gEdkiiPlatformLogoProtocolGuid ## PRODUCES > > > > +[Pcd] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFollowMicrosoftRecommended ## CONSUMES > > + > > [Depex] > > gEfiHiiDatabaseProtocolGuid AND > > gEfiHiiImageExProtocolGuid > > diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec > index be5e829ca9..c8bb51df3b 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -2102,6 +2102,12 @@ > # @Prompt The shared bit mask when Intel Tdx is enabled. > > gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask|0x0|UINT64|0x10000025 > > > > + ## This PCD sets the position of the Boot Logo. > > + # TRUE - The Logo is positioned following the recommendations from Microsoft. > > + # FALSE - The logo is positioned in the center of the screen. > > + # @ Prompt This position of the boot logo > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFollowMicrosoftRecommended|FALSE|BOOLEAN|0x10000026 Which PCD section is this in? This PCD should support all PCD types so it can allow Fixed or Patchable or Dynmaic PCD types. > > + > > [PcdsPatchableInModule] > > ## Specify memory size with page number for PEI code when > > # Loading Module at Fixed Address feature is enabled. > > diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni > index 33ce9f6198..09c1ac1cc1 100644 > --- a/MdeModulePkg/MdeModulePkg.uni > +++ b/MdeModulePkg/MdeModulePkg.uni > @@ -1338,3 +1338,9 @@ > #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPcieResizableBarSupport_HELP #language en-US "Indicates if the PCIe > Resizable BAR Capability Supported.

\n" > > "TRUE - PCIe Resizable BAR > Capability is supported.
\n" > > "FALSE - PCIe Resizable BAR > Capability is not supported.
" > > + > > +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFollowMicrosoftRecommended_PROMPT #language en-US "The position of the Boot > Logo" > > + > > +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFollowMicrosoftRecommend_HELP #language en-US "Sets the position of the > Logo. When set to true, the Logo is positioned following the recommendations" > > + " from Microsoft, 38.2% from > the top of the screen." > > + > > -- > 2.37.2 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#97479): https://edk2.groups.io/g/devel/message/97479 > Mute This Topic: https://groups.io/mt/95697776/1643496 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com] > -=-=-=-=-=-= >