Sheng Lean Tan,

In short, I am looking forward to be convinced by you and Sean, but so far I 
haven’t been.


  1.  In the beginning, I left comments to suggest you use the logic below to 
meet your requirement.
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;
    }


  1.  Then, Sean replied following: “Thank you, it does, and I think it will 
work for most splash images. However, the way it's written in my patch accounts 
for the Image size. This will handle splash images that are equal to, or larger 
than the resolution of the display. “


  1.  Then, I replied “The logic I shared below is from the LogoDxe driver 
which produces EDKII_PLATFORM_LOGO_PROTOCOL. This driver should know the image 
size and it can account for the image size.”
Then, I don’t think we are talking in the same page. Maybe I didn’t understand 
your problem, maybe you didn’t understand my above sample code.
Thanks,
Ray



From: Sheng Lean Tan <sheng....@9elements.com>
Sent: Monday, March 20, 2023 4:12 PM
To: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>; Kinney, Michael D 
<michael.d.kin...@intel.com>
Cc: Rhodes, Sean <sean@starlabs.systems>; Gao, Zhichao <zhichao....@intel.com>; 
Wang, Jian J <jian.j.w...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control 
the position of the Logo

Hi Ray,
Any feedback per Mic feedback?
On 15. Mar 2023, at 16:35, Michael D Kinney 
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> wrote:

HI Ray,

I think it is a reasonable request to have the EDK II logo driver support 
multiple standards for the logo location.  Especially if they are documented in 
public specifications.

The additional conditions of supporting a logo larger than the display 
resolution also looks like a good corner case to cover no matter what logo 
location standard is used.

Perhaps a single PCD that is a enum of logo locations.  Default 0x00 can be EDK 
II default that is centered in the display.  0x01 can be BGRT.  Leaves from for 
more if there are additional public standard logo locations.

Mike


From: Ni, Ray <ray...@intel.com<mailto:ray...@intel.com>>
Sent: Wednesday, March 15, 2023 2:24 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean 
<sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Kinney, Michael D 
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; Gao, Zhichao 
<zhichao....@intel.com<mailto:zhichao....@intel.com>>; Wang, Jian J 
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Subject: RE: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control 
the position of the Logo

What’s the meaning of “have both options”?
If you want to support two cases, put the logic in your platform specific Logo 
driver.
This Logo driver is just for reference.

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: Friday, March 10, 2023 9:43 PM
To: Ni, Ray <ray...@intel.com<mailto:ray...@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D 
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; Gao, Zhichao 
<zhichao....@intel.com<mailto:zhichao....@intel.com>>; Wang, Jian J 
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control 
the position of the Logo

Hi Ray

>                 You can return a carefully-calculated X/Y value to make the 
> logo at MS preferred position.
As we discussed before, we need to have both options.

Thanks

Sean

On Wed, 8 Mar 2023 at 09:01, Ni, Ray 
<ray...@intel.com<mailto:ray...@intel.com>> wrote:
Maybe I didn’t explain my idea clearly.
That is:
                You can get the screen resolution in the code that produces 
Logo protocol.
                You can return a carefully-calculated X/Y value to make the 
logo at MS preferred position.

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> 
<devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray
Sent: Wednesday, October 26, 2022 10:32 AM
To: Kinney, Michael D 
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; 
devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean 
<sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Gao, Zhichao <zhichao....@intel.com<mailto:zhichao....@intel.com>>; Wang, 
Jian J <jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control 
the position of the Logo

Are you suggesting that the exiting logic be updated for this use case without 
adding a new enum?

  1.  yes.

From: Kinney, Michael D 
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray 
<ray...@intel.com<mailto:ray...@intel.com>>; Rhodes, Sean 
<sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D 
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Cc: Gao, Zhichao <zhichao....@intel.com<mailto:zhichao....@intel.com>>; Wang, 
Jian J <jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Subject: RE: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control 
the position of the Logo

Ray,

Are you suggesting that the exiting logic be updated for this use case without 
adding a new enum?

Sean, can you provide a revised patch that does this?

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> 
<devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray
Sent: Tuesday, October 25, 2022 12:58 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean 
<sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Gao, Zhichao <zhichao....@intel.com<mailto:zhichao....@intel.com>>; Wang, 
Jian J <jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control 
the position of the Logo

I need a reason of adding EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended.
In my opinion, without adding this new enum value, it’s still possible to 
support MS recommendation.

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: Tuesday, October 25, 2022 3:27 PM
To: Ni, Ray <ray...@intel.com<mailto:ray...@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao 
<zhichao....@intel.com<mailto:zhichao....@intel.com>>; Wang, Jian J 
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control 
the position of the Logo

Hi Ray

Where would you suggest this code goes? edk2 should support both Microsoft 
recommended and "normal". The original patch handled this well.

Thanks

Sean

On Mon, 10 Oct 2022 at 10:25, Ni, Ray 
<ray...@intel.com<mailto:ray...@intel.com>> wrote:
The logic I shared below is from the LogoDxe driver which produces 
EDKII_PLATFORM_LOGO_PROTOCOL.
This driver should know the image size and it can account for the image size.

Thanks,
Ray

From: Sean Rhodes <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray...@intel.com<mailto:ray...@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao 
<zhichao....@intel.com<mailto:zhichao....@intel.com>>; Wang, Jian J 
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position 
of the Logo

Hi Ray

Thank you, it does, and I think it will work for most splash images. However, 
the way it's written in my patch accounts for the Image size. This will handle 
splash images that are equal to, or larger than the resolution of the display.

Thanks

Sean

On Sat, 8 Oct 2022 at 03:02, Ni, Ray 
<ray...@intel.com<mailto:ray...@intel.com>> wrote:
Sean,
I remember that I evaluated the BGRT requirement when designing the 
PlatformLogo protocol.

So, I went back to got the code I wrote long time ago as below.
I didn't try to understand them now. Does it make sense to you?

    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;
    }

Thanks,
Ray

> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems<mailto:sean@starlabs.systems>>
> Sent: Monday, September 26, 2022 4:10 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Gao, 
> Zhichao
> <zhichao....@intel.com<mailto:zhichao....@intel.com>>; Ni, Ray 
> <ray...@intel.com<mailto:ray...@intel.com>>; Wang, Jian J
> <jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Gao, Liming 
> <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
> Subject: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the
> position of the Logo
>
> When set to true, the Logo is positioned according to the BGRT
> specification, 38.2% from the top of the screen. When set to false,
> no behaviour is changed and the logo is positioned centrally.
>
> Cc: Zhichao Gao <zhichao....@intel.com<mailto:zhichao....@intel.com>>
> Cc: Ray Ni <ray...@intel.com<mailto:ray...@intel.com>>
> Cc: Jian J Wang <jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>
> Cc: Liming Gao <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
> Signed-off-by: Sean Rhodes 
> <sean@starlabs.systems<mailto:sean@starlabs.systems>>
> ---
>  MdeModulePkg/Logo/Logo.c      | 5 +++++
>  MdeModulePkg/Logo/LogoDxe.inf | 4 ++++
>  MdeModulePkg/MdeModulePkg.dec | 6 ++++++
>  MdeModulePkg/MdeModulePkg.uni | 6 ++++++
>  4 files changed, 21 insertions(+)
>
> diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
> index 8ab874d2da..1638d0f984 100644
> --- a/MdeModulePkg/Logo/Logo.c
> +++ b/MdeModulePkg/Logo/Logo.c
> @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Protocol/HiiPackageList.h>
>
>  #include <Library/UefiBootServicesTableLib.h>
>
>  #include <Library/DebugLib.h>
>
> +#include <Library/PcdLib.h>
>
>
>
>  typedef struct {
>
>    EFI_IMAGE_ID                             ImageId;
>
> @@ -69,6 +70,10 @@ GetImage (
>      return EFI_NOT_FOUND;
>
>    }
>
>
>
> +  if (FixedPcdGetBool (PcdFollowMicrosoftRecommended)) {
>
> +    mLogos[Current].Attribute =
> EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended;
>
> +  }
>
> +
>
>    (*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 58e6ab0048..ac437990f1 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|0x
> 10000025
>
>
>
> +  ## 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|FA
> LSE|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_HEL
> P #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_PcdFollowMicrosoftRecommende
> d_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.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101422): https://edk2.groups.io/g/devel/message/101422
Mute This Topic: https://groups.io/mt/93922544/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to