[edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Michael D Kinney
michael.d.kinney at intel.com
Wed Oct 26 22:27:51 UTC 2022
Hi Sean,
That is great feedback!!!
We can discuss with Ray when he is back online on how best to support both policies. You have provides some good options to consider
* Extend enum
* Add PCD
Mike
From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Sean Rhodes
Sent: Wednesday, October 26, 2022 1:30 PM
To: Kinney, Michael D <michael.d.kinney at intel.com>
Cc: Ni, Ray <ray.ni at intel.com>; devel at edk2.groups.io; Gao, Zhichao <zhichao.gao at intel.com>; Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
Hi Mike
This patch is being upstreamed from the coreboot fork; about 70% of the coreboot community wants the logo centred (as the target OS is not Windows), and the rest want to use the 38.2% rule!
Thanks
Sean
On Wed, 26 Oct 2022 at 05:55, Kinney, Michael D <michael.d.kinney at intel.com<mailto:michael.d.kinney at intel.com>> wrote:
Sean provided a sample PR that adds a PCD.
I think we agree that a PCD is not required. The existing logic that centers the logo should be updated to use the 38.2% rule.
Mike
From: Ni, Ray <ray.ni at intel.com<mailto:ray.ni at intel.com>>
Sent: Tuesday, October 25, 2022 7:32 PM
To: Kinney, Michael D <michael.d.kinney at intel.com<mailto:michael.d.kinney at intel.com>>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>; Rhodes, Sean <sean at starlabs.systems<mailto:sean at starlabs.systems>>
Cc: Gao, Zhichao <zhichao.gao at intel.com<mailto:zhichao.gao at intel.com>>; Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at 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 at intel.com<mailto:michael.d.kinney at intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>; Ni, Ray <ray.ni at intel.com<mailto:ray.ni at intel.com>>; Rhodes, Sean <sean at starlabs.systems<mailto:sean at starlabs.systems>>; Kinney, Michael D <michael.d.kinney at intel.com<mailto:michael.d.kinney at intel.com>>
Cc: Gao, Zhichao <zhichao.gao at intel.com<mailto:zhichao.gao at intel.com>>; Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at 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 at edk2.groups.io<mailto:devel at edk2.groups.io> <devel at edk2.groups.io<mailto:devel at edk2.groups.io>> On Behalf Of Ni, Ray
Sent: Tuesday, October 25, 2022 12:58 AM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>; Rhodes, Sean <sean at starlabs.systems<mailto:sean at starlabs.systems>>
Cc: Gao, Zhichao <zhichao.gao at intel.com<mailto:zhichao.gao at intel.com>>; Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at 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 at edk2.groups.io<mailto:devel at edk2.groups.io> <devel at edk2.groups.io<mailto:devel at edk2.groups.io>> On Behalf Of Sean Rhodes
Sent: Tuesday, October 25, 2022 3:27 PM
To: Ni, Ray <ray.ni at intel.com<mailto:ray.ni at intel.com>>
Cc: devel at edk2.groups.io<mailto:devel at edk2.groups.io>; Gao, Zhichao <zhichao.gao at intel.com<mailto:zhichao.gao at intel.com>>; Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at 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 at intel.com<mailto:ray.ni at 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 at starlabs.systems<mailto:sean at starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni at intel.com<mailto:ray.ni at intel.com>>
Cc: devel at edk2.groups.io<mailto:devel at edk2.groups.io>; Gao, Zhichao <zhichao.gao at intel.com<mailto:zhichao.gao at intel.com>>; Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at 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 at intel.com<mailto:ray.ni at 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 at starlabs.systems<mailto:sean at starlabs.systems>>
> Sent: Monday, September 26, 2022 4:10 PM
> To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
> Cc: Rhodes, Sean <sean at starlabs.systems<mailto:sean at starlabs.systems>>; Gao, Zhichao
> <zhichao.gao at intel.com<mailto:zhichao.gao at intel.com>>; Ni, Ray <ray.ni at intel.com<mailto:ray.ni at intel.com>>; Wang, Jian J
> <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at 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 at intel.com<mailto:zhichao.gao at intel.com>>
> Cc: Ray Ni <ray.ni at intel.com<mailto:ray.ni at intel.com>>
> Cc: Jian J Wang <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>
> Cc: Liming Gao <gaoliming at byosoft.com.cn<mailto:gaoliming at byosoft.com.cn>>
> Signed-off-by: Sean Rhodes <sean at starlabs.systems<mailto:sean at 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 (#95623): https://edk2.groups.io/g/devel/message/95623
Mute This Topic: https://groups.io/mt/93922544/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20221026/6bb4a9db/attachment-0001.htm>
More information about the edk2-devel-archive
mailing list