Any feedback per Mic feedba= ck?

On 15. Mar 2023, at = 16:35, Michael D Kinney <michael.d.kinney@intel.com> wrote:

=EF=BB=BF

HI= Ray,

I = think it is a reasonable request to have the EDK II logo driver support mul= tiple standards for the logo location.&nbs= p; Especially if they are documented in public specifications.

Th= e additional conditions of supporting a logo larger than the display resolu= tion also looks like a good corner case to cover no matter what logo location standard is used.

Pe= rhaps a single PCD that is a enum of logo locations. Default 0x00 can be EDK II default that is centered in the display.<= span style=3D"mso-spacerun:yes"> 0x01 can be BGRT. Leav= es from for more if there are additional public standard logo locations.

Mi= ke

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

What=E2=80=99s the meaning of =E2=80=9Chave both opt= ions=E2=80=9D?

If you want to support two cases, put the logic in y= our platform specific Logo driver.

This Logo driver is just for reference.

From: devel@edk2.groups.io <devel@= edk2.groups.io> On Behalf Of Sean Rhodes
Sent: Friday, March 10, 2023 9:43 PM
To: Ni, Ray <ray.ni@intel.com= >
Cc: devel@edk2.groups.io= ; Kinney, Michael D <micha= el.d.kinney@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com<= /a>>; Gao, Liming <gaolim= ing@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD t= o control the position of the Logo

Hi Ray

> &nbs= p; You can return a carefully-calculated X/Y value = to make the logo at MS preferred = position.

As we discussed befo= re, we need to have both options.

Thanks=

Sean

On Wed, 8 Mar 2023 at 09:01, Ni, Ray <ray.ni@intel.com> wrote:

Maybe I didn=E2=80=99t 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: deve= l@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Wednesday, October 26, 2022 10:32 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups= .io; Rhodes, Sean <sean@sta= rlabs.systems>
Cc: Gao, Zhichao <zhichao.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; = Gao, Liming <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD t= o control the position of the Logo

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

yes.

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@= edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Gao, Zhichao <zhichao.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; = Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD t= o 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: deve= l@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Tuesday, October 25, 2022 12:58 AM
To: devel@= edk2.groups.io; Rhodes, Sean <sean@starlabs.systems>
Cc: Gao, Zhichao <zhichao.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; = Gao, Liming <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD t= o control the position of the Logo

I need a reason of adding EdkiiPlatformLogoDisplayAttributeMicroso= ftRecommended.

In my opinion, without adding this new enum value, it=E2=80=99s st= ill possible to support MS recommendation.

From: deve= l@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes
Sent: Tuesday, October 25, 2022 3:27 PM
To: Ni, Ray <ray.ni@intel.com>
Cc: devel@= edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com<= /a>>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD t= o control the position of the Logo

Hi= Ray

&n= bsp;

Wh= ere would you suggest this code goes? edk2 should support both Microsoft re= commended and "normal". The original patch handled this well.

&n= bsp;

Th= anks

&n= bsp;

Se= an

On Mon, 10 Oct 2022 at 10:25, Ni, Ray <ray.ni@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>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com>
Cc: devel@= edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com<= /a>>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: Re: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the= position of the Logo

Hi= Ray

&n= bsp;

Th= ank 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 th= an the resolution of the display.

&n= bsp;

Th= anks

&n= bsp;

Se= an

On Sat, 8 Oct 2022 at 03:02, Ni, Ray <ray.ni@intel.com> wrote:

Sean,
I remember that I evaluated the BGRT requirement when designing the Platfor= mLogo 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 =3D gBS->HandleProtocol (gST->ConsoleOutHandle, = &gEfiGraphicsOutputProtocolGuid, (VOID **) &GraphicsOutput);
if (!EFI_ERROR (Status)) {
//
// Center of LOGO is in the vertical position 38.2% wh= en PcdBootLogoOnlyEnable is TRUE
// Y =3D (VerticalResolution - LogoHeight) / 2
// Y' =3D VerticalResolution * 0.382 - LogoHeight * 0.= 5
// OffsetY + Y =3D Y'
// OffsetY =3D Y' - Y =3D -0.118 * VerticalResolution<= br> //
*Attribute =3D EdkiiPlatformLogoDisplayAttributeCenter= ;
*OffsetX =3D 0;
*OffsetY =3D -118 * (INTN) GraphicsOutput-= >Mode->Info->VerticalResolution / 1000;
}

Thanks,
Ray

> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems>
> Sent: Monday, September 26, 2022 4:10 PM
> To: devel@ed= k2.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: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the
> position of the Logo
>
> When set to true, the Logo is positioned according to the BGRT
> specification, 38.2% from the top of the screen. When set to false, > no behaviour is changed and the logo is positioned centrally.
>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Ray Ni <r= ay.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 | 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 &nb= sp; ImageId;
>
> @@ -69,6 +70,10 @@ GetImage (
> return EFI_NOT_FOUND;
>
> }
>
>
>
> + if (FixedPcdGetBool (PcdFollowMicrosoftRecommended)) {
>
> + mLogos[Current].Attribute =3D
> EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended;
>
> + }
>
> +
>
> (*Instance)++;
>
> *Attribute =3D mLogos[Current].Attribute;
>
> *OffsetX =3D 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 ## PROD= UCES CONSUMES
>
> gEdkiiPlatformLogoProtocolGuid ## PROD= UCES
>
>
>
> +[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.<= br> >
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask|0x0|UINT64|0x
> 10000025
>
>
>
> + ## This PCD sets the position of the Boot Logo.
>
> + # TRUE - The Logo is positioned following th= e 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 enable= d.
>
> 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"
>
> &= nbsp; = &nbs= p; &n= bsp; "TRUE - PCIe Resizable BAR
> Capability is supported.<BR>\n"
>
> &= nbsp; = &nbs= p; &n= bsp; "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"
>
> + = &nbs= p; &n= bsp; = " from Microsoft, 38.2% from
> the top of the screen."
>
> +
>
> --
> 2.34.1

--Apple-Mail-6EE94332-8788-452E-B950-6D9EB693785B--