回复: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo

["gaoliming" <gaoliming@byosoft.com.cn>]

Sean:
  I add my comments below. 

> -----邮件原件-----
> 发件人: Sean Rhodes <sean@starlabs.systems>
> 发送时间: 2022年7月26日 16:15
> 收件人: devel@edk2.groups.io
> 抄送: Sean Rhodes <sean@starlabs.systems>; Zhichao Gao
> <zhichao.gao@intel.com>; Ray Ni <ray.ni@intel.com>; Jian J Wang
> <jian.j.wang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> 主题: [PATCH 2/3] 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.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> ---
>  MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf | 5 ++++-
>  MdeModulePkg/Logo/Logo.c                         | 5 +++++
>  MdeModulePkg/Logo/LogoDxe.inf                    | 4 ++++
>  MdeModulePkg/MdeModulePkg.dec                    | 6 ++++++
>  MdeModulePkg/MdeModulePkg.uni                    | 6 ++++++
>  5 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> index 7d50f2dfa3..14ba8a5906 100644
> --- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> +++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> @@ -48,5 +48,8 @@
>    gEfiUserManagerProtocolGuid                   ## CONSUMES
> 
>    gEdkiiPlatformLogoProtocolGuid                ## CONSUMES
> 
> 
> 
> +[Pcd]
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpecification ##
> CONSUMES
> 
> +
> 
>  [FeaturePcd]
> 
> -  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ## CONSUMES
> 
> +  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport             ##
> CONSUMES
> 
The change in BootLogoLib is not required. Please check. 

> diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
> index 8ab874d2da..73546b32f4 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;
> 
> @@ -69,6 +70,10 @@ GetImage (
>      return EFI_NOT_FOUND;
> 
>    }
> 
> 
> 
> +  if (FixedPcdGetBool (PcdFollowBGRTSpecification)) {
> 
> +    mLogos[Current].Attribute =
> EdkiiPlatformLogoDisplayAttributeBGRTSpecification;
> 
> +  }
> 
> +

Here, please use PcdGetBool(). 

Thanks
Liming
> 
>    (*Instance)++;
> 
>    *Attribute = mLogos[Current].Attribute;
> 
>    *OffsetX   = mLogos[Current].OffsetX;
> 
> diff --git a/MdeModulePkg/Logo/LogoDxe.inf
> b/MdeModulePkg/Logo/LogoDxe.inf
> index 41215d25d8..c5c8ad0bcf 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.PcdFollowBGRTSpecification ##
> CONSUMES
> 
> +
> 
>  [Depex]
> 
>    gEfiHiiDatabaseProtocolGuid AND
> 
>    gEfiHiiImageExProtocolGuid
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 2bcb9f9453..e09918387c 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2095,6 +2095,12 @@
>    # @Prompt The shared bit mask when Intel Tdx is enabled.
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask|0x0|UINT64|0x10
> 000025
> 
> 
> 
> +  ## This PCD sets the position of the Boot Logo.
> 
> +  #   TRUE  - The Logo is positioned according to the BGRT specification.
> 
> +  #   FALSE - The logo is positioned in the center of the screen.
> 
> +  # @ Prompt This position of the boot logo
> 
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpecification|FALSE|BOO
> LEAN|0x10000026
> 
> +
> 
>  [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 b070f15ff2..c6ff7bc1bd 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1334,3 +1334,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_PcdFollowBGRTSpecification_PROM
> PT #language en-US "The position of the Boot Logo"
> 
> +
> 
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFollowBGRTSpecification_HELP
> #language en-US "Sets 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."
> 
> +
> 
> --
> 2.34.1