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

["Sean Rhodes" <sean@starlabs.systems>]

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

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.ni@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 <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@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 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?
>
>    - 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 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 <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 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 <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>; Gao, Liming <gaoliming@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.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>; 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
>
>
>
> 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.ni@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>
> > Sent: Monday, September 26, 2022 4:10 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: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the
> > position of the Logo
> >
> > When set to true, the Logo is positioned according to the BGRT
> > specification, 38.2% from the top of the screen. When set to false,
> > no behaviour is changed and the logo is positioned centrally.
> >
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> > ---
> >  MdeModulePkg/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
>
> 
>

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