From: "Jeshua Smith" <jeshuas@nvidia.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"sean@starlabs.systems" <sean@starlabs.systems>,
"michael.d.kinney@intel.com" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Date: Fri, 16 Dec 2022 15:59:12 +0000 [thread overview]
Message-ID: <DM6PR12MB33711DF70A7A82B215C3DDCCDBE69@DM6PR12MB3371.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CABtds-3XUXkphtbzj2RBy=g5pDwFo6FkTwOWY1r41katnMvM=Q@mail.gmail.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 --]
next prev parent reply other threads:[~2022-12-16 15:59 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-15 21:11 [PATCH] MdeModulePkg/Logo: Add a PCD to control the position of the Logo 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 message]
-- strict thread matches above, loose matches on Subject: below --
2022-12-21 8:19 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR12MB33711DF70A7A82B215C3DDCCDBE69@DM6PR12MB3371.namprd12.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox