public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-12-15 21:11 Sean Rhodes
@ 2022-12-15 22:08 ` Michael D Kinney
  2022-12-15 22:17   ` Sean Rhodes
  0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2022-12-15 22:08 UTC (permalink / raw)
  To: devel@edk2.groups.io, Rhodes, Sean, Kinney, Michael D
  Cc: Gao, Zhichao, Ni, Ray, Wang, Jian J, Gao, Liming

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]
> -=-=-=-=-=-=
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-12-15 22:08 ` [edk2-devel] " Michael D Kinney
@ 2022-12-15 22:17   ` Sean Rhodes
  2022-12-15 22:54     ` Michael D Kinney
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Rhodes @ 2022-12-15 22:17 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: devel@edk2.groups.io, Gao, Zhichao, Ni, Ray, Wang, Jian J,
	Gao, Liming

[-- Attachment #1: Type: text/plain, Size: 7645 bytes --]

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]
> > -=-=-=-=-=-=
> >
>
>

[-- Attachment #2: Type: text/html, Size: 12403 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-12-15 22:17   ` Sean Rhodes
@ 2022-12-15 22:54     ` Michael D Kinney
  2022-12-16  8:57       ` Sean Rhodes
  0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2022-12-15 22:54 UTC (permalink / raw)
  To: Rhodes, Sean, Kinney, Michael D
  Cc: devel@edk2.groups.io, Gao, Zhichao, Ni, Ray, Wang, Jian J,
	Gao, Liming

[-- Attachment #1: Type: text/plain, Size: 8958 bytes --]

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>]
> -=-=-=-=-=-=
>

[-- Attachment #2: Type: text/html, Size: 56247 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-12-15 22:54     ` Michael D Kinney
@ 2022-12-16  8:57       ` Sean Rhodes
  2022-12-16 15:59         ` Jeshua Smith
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Rhodes @ 2022-12-16  8:57 UTC (permalink / raw)
  To: devel, michael.d.kinney

[-- Attachment #1: Type: text/plain, Size: 8804 bytes --]

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]
> > -=-=-=-=-=-=
> >
>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 16788 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-12-16  8:57       ` Sean Rhodes
@ 2022-12-16 15:59         ` Jeshua Smith
  0 siblings, 0 replies; 12+ messages in thread
From: Jeshua Smith @ 2022-12-16 15:59 UTC (permalink / raw)
  To: devel@edk2.groups.io, sean@starlabs.systems,
	michael.d.kinney@intel.com

[-- Attachment #1: Type: text/plain, Size: 9880 bytes --]

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>]
> -=-=-=-=-=-=
>


[-- Attachment #2: Type: text/html, Size: 23010 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
@ 2022-12-21  8:19 Sean Rhodes
  2023-02-17 12:58 ` [edk2-devel] " Sheng Lean Tan
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Sean Rhodes @ 2022-12-21  8:19 UTC (permalink / raw)
  To: devel; +Cc: Sean Rhodes, Zhichao Gao, Ray Ni, Jian J Wang, Liming Gao

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/MdeModulePkg.dec |  6 ++++++
 MdeModulePkg/Logo/LogoDxe.inf |  4 ++++
 MdeModulePkg/Logo/Logo.c      | 28 +++++++++++++++++++++++++++-
 MdeModulePkg/MdeModulePkg.uni |  6 ++++++
 4 files changed, 43 insertions(+), 1 deletion(-)

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 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   # @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/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 @@ [LibraryClasses]
   UefiBootServicesTableLib
   UefiDriverEntryPoint
   DebugLib
+  PcdLib
 
 [Protocols]
   gEfiHiiDatabaseProtocolGuid        ## CONSUMES
@@ -48,6 +49,9 @@ [Protocols]
   gEfiHiiPackageListProtocolGuid     ## PRODUCES CONSUMES
   gEdkiiPlatformLogoProtocolGuid     ## PRODUCES
 
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowMicrosoftRecommended ## CONSUMES
+
 [Depex]
   gEfiHiiDatabaseProtocolGuid AND
   gEfiHiiImageExProtocolGuid
diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
index 8ab874d2da..96e34b2011 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 (PcdGetBool (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/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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-12-21  8:19 [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
@ 2023-02-17 12:58 ` Sheng Lean Tan
       [not found] ` <17449E1187E223D3.28744@groups.io>
  2023-03-08  8:59 ` Sheng Lean Tan
  2 siblings, 0 replies; 12+ messages in thread
From: Sheng Lean Tan @ 2023-02-17 12:58 UTC (permalink / raw)
  To: devel, sean; +Cc: Zhichao Gao, Ray Ni, Jian J Wang, Liming Gao

[-- Attachment #1: Type: text/plain, Size: 6593 bytes --]

Hi all,
Could you help to review this? thanks.

Best Regards,
*Lean Sheng Tan*



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: sheng.tan@9elements.com
Phone: *+49 234 68 94 188 <+492346894188>*
Mobile: *+49 176 76 113842 <+4917676113842>*

Registered office: Bochum
Commercial register: Amtsgericht Bochum, HRB 17519
Management: Sebastian German, Eray Bazaar

Data protection information according to Art. 13 GDPR
<https://9elements.com/privacy>


On Wed, 21 Dec 2022 at 09:20, Sean Rhodes <sean@starlabs.systems> wrote:

> 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/MdeModulePkg.dec |  6 ++++++
>  MdeModulePkg/Logo/LogoDxe.inf |  4 ++++
>  MdeModulePkg/Logo/Logo.c      | 28 +++++++++++++++++++++++++++-
>  MdeModulePkg/MdeModulePkg.uni |  6 ++++++
>  4 files changed, 43 insertions(+), 1 deletion(-)
>
> 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 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic, PcdsDynamicEx]
>    # @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/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 @@ [LibraryClasses]
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
>    DebugLib
> +  PcdLib
>
>  [Protocols]
>    gEfiHiiDatabaseProtocolGuid        ## CONSUMES
> @@ -48,6 +49,9 @@ [Protocols]
>    gEfiHiiPackageListProtocolGuid     ## PRODUCES CONSUMES
>    gEdkiiPlatformLogoProtocolGuid     ## PRODUCES
>
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowMicrosoftRecommended ## CONSUMES
> +
>  [Depex]
>    gEfiHiiDatabaseProtocolGuid AND
>    gEfiHiiImageExProtocolGuid
> diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
> index 8ab874d2da..96e34b2011 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 (PcdGetBool (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/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 (#97684): https://edk2.groups.io/g/devel/message/97684
> Mute This Topic: https://groups.io/mt/95802829/6757431
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [sheng.tan@9elements.com
> ]
> ------------
>
>
>

[-- Attachment #2: Type: text/html, Size: 10506 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
       [not found] ` <17449E1187E223D3.28744@groups.io>
@ 2023-03-06  9:56   ` Sheng Lean Tan
  2023-03-07 11:32     ` Sheng Lean Tan
  0 siblings, 1 reply; 12+ messages in thread
From: Sheng Lean Tan @ 2023-03-06  9:56 UTC (permalink / raw)
  To: devel, sheng.tan; +Cc: sean, Zhichao Gao, Ray Ni, Jian J Wang, Liming Gao

[-- Attachment #1: Type: text/plain, Size: 7431 bytes --]

This patch has been pending for ages, can someone help to review?
Thank you.
Best Regards,
*Lean Sheng Tan*



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: sheng.tan@9elements.com
Phone: *+49 234 68 94 188 <+492346894188>*
Mobile: *+49 176 76 113842 <+4917676113842>*

Registered office: Bochum
Commercial register: Amtsgericht Bochum, HRB 17519
Management: Sebastian German, Eray Bazaar

Data protection information according to Art. 13 GDPR
<https://9elements.com/privacy>


On Fri, 17 Feb 2023 at 13:59, Sheng Lean Tan via groups.io <sheng.tan=
9elements.com@groups.io> wrote:

> Hi all,
> Could you help to review this? thanks.
>
> Best Regards,
> *Lean Sheng Tan*
>
>
>
> 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
> Email: sheng.tan@9elements.com
> Phone: *+49 234 68 94 188 <+492346894188>*
> Mobile: *+49 176 76 113842 <+4917676113842>*
>
> Registered office: Bochum
> Commercial register: Amtsgericht Bochum, HRB 17519
> Management: Sebastian German, Eray Bazaar
>
> Data protection information according to Art. 13 GDPR
> <https://9elements.com/privacy>
>
>
> On Wed, 21 Dec 2022 at 09:20, Sean Rhodes <sean@starlabs.systems> wrote:
>
>> 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/MdeModulePkg.dec |  6 ++++++
>>  MdeModulePkg/Logo/LogoDxe.inf |  4 ++++
>>  MdeModulePkg/Logo/Logo.c      | 28 +++++++++++++++++++++++++++-
>>  MdeModulePkg/MdeModulePkg.uni |  6 ++++++
>>  4 files changed, 43 insertions(+), 1 deletion(-)
>>
>> 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 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
>> PcdsDynamic, PcdsDynamicEx]
>>    # @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/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 @@ [LibraryClasses]
>>    UefiBootServicesTableLib
>>    UefiDriverEntryPoint
>>    DebugLib
>> +  PcdLib
>>
>>  [Protocols]
>>    gEfiHiiDatabaseProtocolGuid        ## CONSUMES
>> @@ -48,6 +49,9 @@ [Protocols]
>>    gEfiHiiPackageListProtocolGuid     ## PRODUCES CONSUMES
>>    gEdkiiPlatformLogoProtocolGuid     ## PRODUCES
>>
>> +[Pcd]
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowMicrosoftRecommended ##
>> CONSUMES
>> +
>>  [Depex]
>>    gEfiHiiDatabaseProtocolGuid AND
>>    gEfiHiiImageExProtocolGuid
>> diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
>> index 8ab874d2da..96e34b2011 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 (PcdGetBool (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/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 (#97684): https://edk2.groups.io/g/devel/message/97684
>> Mute This Topic: https://groups.io/mt/95802829/6757431
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [
>> sheng.tan@9elements.com]
>> ------------
>>
>>
>> 
>
>

[-- Attachment #2: Type: text/html, Size: 13392 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-06  9:56   ` Sheng Lean Tan
@ 2023-03-07 11:32     ` Sheng Lean Tan
  2023-03-08  8:55       ` Sheng Lean Tan
  0 siblings, 1 reply; 12+ messages in thread
From: Sheng Lean Tan @ 2023-03-07 11:32 UTC (permalink / raw)
  To: Sheng Lean Tan, devel

[-- Attachment #1: Type: text/plain, Size: 32 bytes --]

Another kind reminder, thanks.

[-- Attachment #2: Type: text/html, Size: 32 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-07 11:32     ` Sheng Lean Tan
@ 2023-03-08  8:55       ` Sheng Lean Tan
  2023-03-08  9:01         ` Sheng Lean Tan
  0 siblings, 1 reply; 12+ messages in thread
From: Sheng Lean Tan @ 2023-03-08  8:55 UTC (permalink / raw)
  To: Sheng Lean Tan, devel

[-- Attachment #1: Type: text/plain, Size: 237 bytes --]

Same here.
Seriously, what is the hold up here?
did Sean not following the process? Did he miss anything?
it just doesn’t make sense to keep ignoring this patch for half year for no reason. Is there a way to voice up a about this?

[-- Attachment #2: Type: text/html, Size: 253 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-12-21  8:19 [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
  2023-02-17 12:58 ` [edk2-devel] " Sheng Lean Tan
       [not found] ` <17449E1187E223D3.28744@groups.io>
@ 2023-03-08  8:59 ` Sheng Lean Tan
  2 siblings, 0 replies; 12+ messages in thread
From: Sheng Lean Tan @ 2023-03-08  8:59 UTC (permalink / raw)
  To: Sean Rhodes, devel

[-- Attachment #1: Type: text/plain, Size: 124 bytes --]

It also doesn’t give good impression on edk2 if the reviewers continue to ignore review requests after such a long time.

[-- Attachment #2: Type: text/html, Size: 128 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-08  8:55       ` Sheng Lean Tan
@ 2023-03-08  9:01         ` Sheng Lean Tan
  0 siblings, 0 replies; 12+ messages in thread
From: Sheng Lean Tan @ 2023-03-08  9:01 UTC (permalink / raw)
  To: Sheng Lean Tan, devel, Wang Jian J, Gao Liming, Gao Zhichao,
	Ni Ray



On 8. Mar 2023, at 09:55, Sheng Lean Tan <sheng.tan@9elements.com> wrote:

Same here.
Seriously, what is the hold up here?
did Sean not following the process? Did he miss anything?
it just doesn’t make sense to keep ignoring this patch for half year for no reason. Is there a way to voice up a about this?

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-03-08  9:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-21  8:19 [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
2023-02-17 12:58 ` [edk2-devel] " Sheng Lean Tan
     [not found] ` <17449E1187E223D3.28744@groups.io>
2023-03-06  9:56   ` Sheng Lean Tan
2023-03-07 11:32     ` Sheng Lean Tan
2023-03-08  8:55       ` Sheng Lean Tan
2023-03-08  9:01         ` Sheng Lean Tan
2023-03-08  8:59 ` Sheng Lean Tan
  -- strict thread matches above, loose matches on Subject: below --
2022-12-15 21:11 Sean Rhodes
2022-12-15 22:08 ` [edk2-devel] " Michael D Kinney
2022-12-15 22:17   ` Sean Rhodes
2022-12-15 22:54     ` Michael D Kinney
2022-12-16  8:57       ` Sean Rhodes
2022-12-16 15:59         ` Jeshua Smith

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox