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 <michael.d.kin...@intel.com>
wrote:
> Hi Sean,
>
> A couple comments related to the PCD type below.
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <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 <sean@starlabs.systems>; Gao, Zhichao <
> zhichao....@intel.com>; Ni, Ray <ray...@intel.com>; Wang, Jian J
> > <jian.j.w...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>
> > 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 <zhichao....@intel.com>
> > Cc: Ray Ni <ray...@intel.com>
> > Cc: Jian J Wang <jian.j.w...@intel.com>
> > Cc: Liming Gao <gaolim...@byosoft.com.cn>
> > Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> > ---
> > 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 <Protocol/HiiPackageList.h>
> >
> > #include <Library/UefiBootServicesTableLib.h>
> >
> > #include <Library/DebugLib.h>
> >
> > +#include <Library/PcdLib.h>
> >
> >
> >
> > 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.<BR><BR>\n"
> >
> >
> "TRUE - PCIe Resizable BAR
> > Capability is supported.<BR>\n"
> >
> >
> "FALSE - PCIe Resizable BAR
> > Capability is not supported.<BR>"
> >
> > +
> >
> > +#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+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [
> michael.d.kin...@intel.com]
> > -=-=-=-=-=-=
> >
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97481): https://edk2.groups.io/g/devel/message/97481
Mute This Topic: https://groups.io/mt/95697776/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
