[edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo

Sean Rhodes posted 1 patch 1 year, 4 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
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(-)
[edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Posted by Sean Rhodes 1 year, 4 months ago
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/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)) {
+    //
+    // 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
+
 [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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Posted by Michael D Kinney 1 year, 4 months ago
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.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Gao, Liming <gaoliming@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.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/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+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97480): https://edk2.groups.io/g/devel/message/97480
Mute This Topic: https://groups.io/mt/95697776/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Posted by Sean Rhodes 1 year, 4 months ago
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.kinney@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.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Gao, Liming <gaoliming@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.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/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+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [
> michael.d.kinney@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Posted by Michael D Kinney 1 year, 4 months ago
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 <sean@starlabs.systems>
Sent: Thursday, December 15, 2022 2:17 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
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 <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Sean,

A couple comments related to the PCD type below.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean Rhodes
> Sent: Thursday, December 15, 2022 1:12 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Wang, Jian J
> <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@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.gao@intel.com<mailto:zhichao.gao@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
> Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems<mailto: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+owner@edk2.groups.io<mailto:devel%2Bowner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>]
> -=-=-=-=-=-=
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97482): https://edk2.groups.io/g/devel/message/97482
Mute This Topic: https://groups.io/mt/95697776/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Posted by Sean Rhodes 1 year, 4 months ago
Hi Mike

Thanks; didn't work but I'll have a play wth it!

Sean

On Thu, 15 Dec 2022 at 22:55, Michael D Kinney <michael.d.kinney@intel.com>
wrote:

> 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 <sean@starlabs.systems>
> *Sent:* Thursday, December 15, 2022 2:17 PM
> *To:* Kinney, Michael D <michael.d.kinney@intel.com>
> *Cc:* devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <
> gaoliming@byosoft.com.cn>
> *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 <
> michael.d.kinney@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.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Gao, Liming <gaoliming@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.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/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+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [
> michael.d.kinney@intel.com]
> > -=-=-=-=-=-=
> >
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97495): https://edk2.groups.io/g/devel/message/97495
Mute This Topic: https://groups.io/mt/95697776/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Posted by Jeshua Smith via groups.io 1 year, 4 months ago
I haven’t tried it, but farther down on the page steps 9 and 10 look like they’re related to the option Mike suggested, so they might be required for it to work?


From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes via groups.io
Sent: Friday, December 16, 2022 1:58 AM
To: devel@edk2.groups.io; michael.d.kinney@intel.com
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo

External email: Use caution opening links or attachments

Hi Mike

Thanks; didn't work but I'll have a play wth it!

Sean

On Thu, 15 Dec 2022 at 22:55, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
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 <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Sent: Thursday, December 15, 2022 2:17 PM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
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 <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Sean,

A couple comments related to the PCD type below.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean Rhodes
> Sent: Thursday, December 15, 2022 1:12 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Wang, Jian J
> <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@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.gao@intel.com<mailto:zhichao.gao@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
> Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems<mailto: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+owner@edk2.groups.io<mailto:devel%2Bowner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>]
> -=-=-=-=-=-=
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97510): https://edk2.groups.io/g/devel/message/97510
Mute This Topic: https://groups.io/mt/95697776/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-