public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* 回复: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-07-26  8:15 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
@ 2022-08-05  5:58   ` gaoliming
  0 siblings, 0 replies; 30+ messages in thread
From: gaoliming @ 2022-08-05  5:58 UTC (permalink / raw)
  To: 'Sean Rhodes', devel
  Cc: 'Zhichao Gao', 'Ray Ni', 'Jian J Wang'

Sean:
  I add my comments below. 

> -----邮件原件-----
> 发件人: Sean Rhodes <sean@starlabs.systems>
> 发送时间: 2022年7月26日 16:15
> 收件人: devel@edk2.groups.io
> 抄送: Sean Rhodes <sean@starlabs.systems>; Zhichao Gao
> <zhichao.gao@intel.com>; Ray Ni <ray.ni@intel.com>; Jian J Wang
> <jian.j.wang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> 主题: [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/Library/BootLogoLib/BootLogoLib.inf | 5 ++++-
>  MdeModulePkg/Logo/Logo.c                         | 5 +++++
>  MdeModulePkg/Logo/LogoDxe.inf                    | 4 ++++
>  MdeModulePkg/MdeModulePkg.dec                    | 6 ++++++
>  MdeModulePkg/MdeModulePkg.uni                    | 6 ++++++
>  5 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> index 7d50f2dfa3..14ba8a5906 100644
> --- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> +++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> @@ -48,5 +48,8 @@
>    gEfiUserManagerProtocolGuid                   ## CONSUMES
> 
>    gEdkiiPlatformLogoProtocolGuid                ## CONSUMES
> 
> 
> 
> +[Pcd]
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpecification ##
> CONSUMES
> 
> +
> 
>  [FeaturePcd]
> 
> -  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ## CONSUMES
> 
> +  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport             ##
> CONSUMES
> 
The change in BootLogoLib is not required. Please check. 

> diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
> index 8ab874d2da..73546b32f4 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 (PcdFollowBGRTSpecification)) {
> 
> +    mLogos[Current].Attribute =
> EdkiiPlatformLogoDisplayAttributeBGRTSpecification;
> 
> +  }
> 
> +

Here, please use PcdGetBool(). 

Thanks
Liming
> 
>    (*Instance)++;
> 
>    *Attribute = mLogos[Current].Attribute;
> 
>    *OffsetX   = mLogos[Current].OffsetX;
> 
> diff --git a/MdeModulePkg/Logo/LogoDxe.inf
> b/MdeModulePkg/Logo/LogoDxe.inf
> index 41215d25d8..c5c8ad0bcf 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.PcdFollowBGRTSpecification ##
> CONSUMES
> 
> +
> 
>  [Depex]
> 
>    gEfiHiiDatabaseProtocolGuid AND
> 
>    gEfiHiiImageExProtocolGuid
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 2bcb9f9453..e09918387c 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2095,6 +2095,12 @@
>    # @Prompt The shared bit mask when Intel Tdx is enabled.
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask|0x0|UINT64|0x10
> 000025
> 
> 
> 
> +  ## This PCD sets the position of the Boot Logo.
> 
> +  #   TRUE  - The Logo is positioned according to the BGRT specification.
> 
> +  #   FALSE - The logo is positioned in the center of the screen.
> 
> +  # @ Prompt This position of the boot logo
> 
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpecification|FALSE|BOO
> LEAN|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 b070f15ff2..c6ff7bc1bd 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1334,3 +1334,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_PcdFollowBGRTSpecification_PROM
> PT #language en-US "The position of the Boot Logo"
> 
> +
> 
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFollowBGRTSpecification_HELP
> #language en-US "Sets 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."
> 
> +
> 
> --
> 2.34.1




^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations
@ 2022-09-26  8:09 Sean Rhodes
  2022-09-26  8:09 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
                   ` (4 more replies)
  0 siblings, 5 replies; 30+ messages in thread
From: Sean Rhodes @ 2022-09-26  8:09 UTC (permalink / raw)
  To: devel; +Cc: Sean Rhodes, Zhichao Gao, Ray Ni, Jian J Wang, Liming Gao

Add an option to position the logo 38.2% from the top of the screen,
which follows the recommendations from Microsoft. These can be found
here:
https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-screen-components

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/Include/Protocol/PlatformLogo.h   | 3 ++-
 MdeModulePkg/Library/BootLogoLib/BootLogoLib.c | 7 ++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h b/MdeModulePkg/Include/Protocol/PlatformLogo.h
index 08e1dc35a4..b24d7d5b79 100644
--- a/MdeModulePkg/Include/Protocol/PlatformLogo.h
+++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h
@@ -29,7 +29,8 @@ typedef enum {
   EdkiiPlatformLogoDisplayAttributeCenterBottom,
   EdkiiPlatformLogoDisplayAttributeLeftBottom,
   EdkiiPlatformLogoDisplayAttributeCenterLeft,
-  EdkiiPlatformLogoDisplayAttributeCenter
+  EdkiiPlatformLogoDisplayAttributeCenter,
+  EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended
 } EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE;
 
 /**
diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
index 478ec2d40e..9065e5281b 100644
--- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
+++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
@@ -169,7 +169,6 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;
         DestY = 0;
         break;
-
       case EdkiiPlatformLogoDisplayAttributeCenterLeft:
         DestX = 0;
         DestY = (SizeOfY - Image.Height) / 2;
@@ -182,7 +181,6 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;
         DestY = (SizeOfY - Image.Height) / 2;
         break;
-
       case EdkiiPlatformLogoDisplayAttributeLeftBottom:
         DestX = 0;
         DestY = SizeOfY - Image.Height;
@@ -195,7 +193,10 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;
         DestY = SizeOfY - Image.Height;
         break;
-
+      case EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended:
+        DestX = (SizeOfX - Image.Width) / 2;
+        DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;
+        break;
       default:
         ASSERT (FALSE);
         continue;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-09-26  8:09 [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations Sean Rhodes
@ 2022-09-26  8:09 ` Sean Rhodes
  2022-10-08  1:37   ` 回复: " gaoliming
  2022-10-08  2:02   ` Ni, Ray
  2022-09-26  8:09 ` [PATCH 3/3] UefiPayloadPkg: Hook up MICROSOFT_RECOMMENDED macro to PcdFollowMicrosoftRecommended Sean Rhodes
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 30+ messages in thread
From: Sean Rhodes @ 2022-09-26  8:09 UTC (permalink / raw)
  To: devel; +Cc: Sean Rhodes, Zhichao Gao, Ray Ni, Jian J Wang, Liming Gao

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|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
+
 [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.34.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/3] UefiPayloadPkg: Hook up MICROSOFT_RECOMMENDED macro to PcdFollowMicrosoftRecommended.
  2022-09-26  8:09 [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations Sean Rhodes
  2022-09-26  8:09 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
@ 2022-09-26  8:09 ` Sean Rhodes
  2022-10-03  9:22 ` [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations Sheng Lean Tan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Sean Rhodes @ 2022-09-26  8:09 UTC (permalink / raw)
  To: devel; +Cc: Sean Rhodes, Guo Dong, Ray Ni

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 8f23802199..48b2a650f2 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -43,6 +43,7 @@
   DEFINE SD_MMC_TIMEOUT               = 1000000
   DEFINE USE_CBMEM_FOR_CONSOLE        = FALSE
   DEFINE BOOTSPLASH_IMAGE             = FALSE
+  DEFINE MICROSOFT_RECOMMENDED        = FALSE
   DEFINE NVME_ENABLE                  = TRUE
 
   #
@@ -444,6 +445,7 @@
 
   gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory|$(ABOVE_4G_MEMORY)
   gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|$(BOOT_MANAGER_ESCAPE)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowMicrosoftRecommended|$(MICROSOFT_RECOMMENDED)
 
   gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1800000
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations
  2022-09-26  8:09 [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations Sean Rhodes
  2022-09-26  8:09 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
  2022-09-26  8:09 ` [PATCH 3/3] UefiPayloadPkg: Hook up MICROSOFT_RECOMMENDED macro to PcdFollowMicrosoftRecommended Sean Rhodes
@ 2022-10-03  9:22 ` Sheng Lean Tan
       [not found] ` <171A84BCA8A12F72.28182@groups.io>
  2022-10-08  1:37 ` 回复: " gaoliming
  4 siblings, 0 replies; 30+ messages in thread
From: Sheng Lean Tan @ 2022-10-03  9:22 UTC (permalink / raw)
  To: devel@edk2.groups.io, sean@starlabs.systems
  Cc: Sean Rhodes, Zhichao Gao, Ray Ni, Jian J Wang, Liming Gao,
	Dong, Guo, Guo, Gua

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

Hi Liming,
Mind to help to take a look at these 2 patches? Sean has updated the initial patches per your recommendation:
https://edk2.groups.io/g/devel/message/94318?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C93922542

https://edk2.groups.io/g/devel/message/94320?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C93922544

Previous feedback:
https://edk2.groups.io/g/devel/message/92385?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C92974182

https://edk2.groups.io/g/devel/message/92146?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C92830014

Thanks,
Sheng

From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Sean Rhodes <sean@starlabs.systems>
Date: Monday, 26. September 2022 at 10:10
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sean Rhodes <sean@starlabs.systems>, Zhichao Gao <zhichao.gao@intel.com>, Ray Ni <ray.ni@intel.com>, Jian J Wang <jian.j.wang@intel.com>, Liming Gao <gaoliming@byosoft.com.cn>
Subject: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations
Add an option to position the logo 38.2% from the top of the screen,
which follows the recommendations from Microsoft. These can be found
here:
https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-screen-components

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/Include/Protocol/PlatformLogo.h   | 3 ++-
 MdeModulePkg/Library/BootLogoLib/BootLogoLib.c | 7 ++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h b/MdeModulePkg/Include/Protocol/PlatformLogo.h
index 08e1dc35a4..b24d7d5b79 100644
--- a/MdeModulePkg/Include/Protocol/PlatformLogo.h
+++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h
@@ -29,7 +29,8 @@ typedef enum {
   EdkiiPlatformLogoDisplayAttributeCenterBottom,

   EdkiiPlatformLogoDisplayAttributeLeftBottom,

   EdkiiPlatformLogoDisplayAttributeCenterLeft,

-  EdkiiPlatformLogoDisplayAttributeCenter

+  EdkiiPlatformLogoDisplayAttributeCenter,

+  EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended

 } EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE;



 /**

diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
index 478ec2d40e..9065e5281b 100644
--- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
+++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
@@ -169,7 +169,6 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;

         DestY = 0;

         break;

-

       case EdkiiPlatformLogoDisplayAttributeCenterLeft:

         DestX = 0;

         DestY = (SizeOfY - Image.Height) / 2;

@@ -182,7 +181,6 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;

         DestY = (SizeOfY - Image.Height) / 2;

         break;

-

       case EdkiiPlatformLogoDisplayAttributeLeftBottom:

         DestX = 0;

         DestY = SizeOfY - Image.Height;

@@ -195,7 +193,10 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;

         DestY = SizeOfY - Image.Height;

         break;

-

+      case EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended:

+        DestX = (SizeOfX - Image.Width) / 2;

+        DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;

+        break;

       default:

         ASSERT (FALSE);

         continue;

--
2.34.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94318): https://edk2.groups.io/g/devel/message/94318
Mute This Topic: https://groups.io/mt/93922542/6757431
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [sheng.tan@9elements.com]
-=-=-=-=-=-=


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations
       [not found] ` <171A84BCA8A12F72.28182@groups.io>
@ 2022-10-07  9:14   ` Sheng Lean Tan
  0 siblings, 0 replies; 30+ messages in thread
From: Sheng Lean Tan @ 2022-10-07  9:14 UTC (permalink / raw)
  To: devel@edk2.groups.io

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

Reminder for review, thanks.

From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Sheng Lean Tan via groups.io <sheng.tan=9elements.com@groups.io>
Date: Monday, 3. October 2022 at 11:22
To: devel@edk2.groups.io <devel@edk2.groups.io>, sean@starlabs.systems <sean@starlabs.systems>
Cc: Sean Rhodes <sean@starlabs.systems>, Zhichao Gao <zhichao.gao@intel.com>, Ray Ni <ray.ni@intel.com>, Jian J Wang <jian.j.wang@intel.com>, Liming Gao <gaoliming@byosoft.com.cn>, Dong, Guo <guo.dong@intel.com>, Guo, Gua <gua.guo@intel.com>
Subject: Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations
Hi Liming,
Mind to help to take a look at these 2 patches? Sean has updated the initial patches per your recommendation:
https://edk2.groups.io/g/devel/message/94318?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C93922542

https://edk2.groups.io/g/devel/message/94320?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C93922544

Previous feedback:
https://edk2.groups.io/g/devel/message/92385?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C92974182

https://edk2.groups.io/g/devel/message/92146?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cboot+logo+sean%2C20%2C2%2C0%2C92830014

Thanks,
Sheng

From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Sean Rhodes <sean@starlabs.systems>
Date: Monday, 26. September 2022 at 10:10
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sean Rhodes <sean@starlabs.systems>, Zhichao Gao <zhichao.gao@intel.com>, Ray Ni <ray.ni@intel.com>, Jian J Wang <jian.j.wang@intel.com>, Liming Gao <gaoliming@byosoft.com.cn>
Subject: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations
Add an option to position the logo 38.2% from the top of the screen,
which follows the recommendations from Microsoft. These can be found
here:
https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-screen-components

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/Include/Protocol/PlatformLogo.h   | 3 ++-
 MdeModulePkg/Library/BootLogoLib/BootLogoLib.c | 7 ++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h b/MdeModulePkg/Include/Protocol/PlatformLogo.h
index 08e1dc35a4..b24d7d5b79 100644
--- a/MdeModulePkg/Include/Protocol/PlatformLogo.h
+++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h
@@ -29,7 +29,8 @@ typedef enum {
   EdkiiPlatformLogoDisplayAttributeCenterBottom,

   EdkiiPlatformLogoDisplayAttributeLeftBottom,

   EdkiiPlatformLogoDisplayAttributeCenterLeft,

-  EdkiiPlatformLogoDisplayAttributeCenter

+  EdkiiPlatformLogoDisplayAttributeCenter,

+  EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended

 } EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE;



 /**

diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
index 478ec2d40e..9065e5281b 100644
--- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
+++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
@@ -169,7 +169,6 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;

         DestY = 0;

         break;

-

       case EdkiiPlatformLogoDisplayAttributeCenterLeft:

         DestX = 0;

         DestY = (SizeOfY - Image.Height) / 2;

@@ -182,7 +181,6 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;

         DestY = (SizeOfY - Image.Height) / 2;

         break;

-

       case EdkiiPlatformLogoDisplayAttributeLeftBottom:

         DestX = 0;

         DestY = SizeOfY - Image.Height;

@@ -195,7 +193,10 @@ BootLogoEnableLogo (
         DestX = SizeOfX - Image.Width;

         DestY = SizeOfY - Image.Height;

         break;

-

+      case EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended:

+        DestX = (SizeOfX - Image.Width) / 2;

+        DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;

+        break;

       default:

         ASSERT (FALSE);

         continue;

--
2.34.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94318): https://edk2.groups.io/g/devel/message/94318
Mute This Topic: https://groups.io/mt/93922542/6757431
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [sheng.tan@9elements.com]
-=-=-=-=-=-=




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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* 回复: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-09-26  8:09 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
@ 2022-10-08  1:37   ` gaoliming
  2022-10-08  2:02   ` Ni, Ray
  1 sibling, 0 replies; 30+ messages in thread
From: gaoliming @ 2022-10-08  1:37 UTC (permalink / raw)
  To: 'Sean Rhodes', devel
  Cc: 'Zhichao Gao', 'Ray Ni', 'Jian J Wang'

Sean:
  Thanks for your update. I have one minor comment in below. Other change is
good to me. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Sean Rhodes <sean@starlabs.systems>
> 发送时间: 2022年9月26日 16:10
> 收件人: devel@edk2.groups.io
> 抄送: Sean Rhodes <sean@starlabs.systems>; Zhichao Gao
> <zhichao.gao@intel.com>; Ray Ni <ray.ni@intel.com>; Jian J Wang
> <jian.j.wang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> 主题: [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;
> 
> +  }
> 
> +

[Liming] Please use PcdGetBool (PcdFollowMicrosoftRecommended), 
PcdGetBool is more flexible for PCD usage. 

Thanks
Liming
> 
>    (*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|0x10
> 000025
> 
> 
> 
> +  ## 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
> 
> +
> 
>  [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_HE
> LP   #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




^ permalink raw reply	[flat|nested] 30+ messages in thread

* 回复: [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations
  2022-09-26  8:09 [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations Sean Rhodes
                   ` (3 preceding siblings ...)
       [not found] ` <171A84BCA8A12F72.28182@groups.io>
@ 2022-10-08  1:37 ` gaoliming
  4 siblings, 0 replies; 30+ messages in thread
From: gaoliming @ 2022-10-08  1:37 UTC (permalink / raw)
  To: 'Sean Rhodes', devel
  Cc: 'Zhichao Gao', 'Ray Ni', 'Jian J Wang'

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Sean Rhodes <sean@starlabs.systems>
> 发送时间: 2022年9月26日 16:10
> 收件人: devel@edk2.groups.io
> 抄送: Sean Rhodes <sean@starlabs.systems>; Zhichao Gao
> <zhichao.gao@intel.com>; Ray Ni <ray.ni@intel.com>; Jian J Wang
> <jian.j.wang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> 主题: [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow
> Microsoft Recommendations
> 
> Add an option to position the logo 38.2% from the top of the screen,
> which follows the recommendations from Microsoft. These can be found
> here:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-s
> creen-components
> 
> 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/Include/Protocol/PlatformLogo.h   | 3 ++-
>  MdeModulePkg/Library/BootLogoLib/BootLogoLib.c | 7 ++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h
> b/MdeModulePkg/Include/Protocol/PlatformLogo.h
> index 08e1dc35a4..b24d7d5b79 100644
> --- a/MdeModulePkg/Include/Protocol/PlatformLogo.h
> +++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h
> @@ -29,7 +29,8 @@ typedef enum {
>    EdkiiPlatformLogoDisplayAttributeCenterBottom,
> 
>    EdkiiPlatformLogoDisplayAttributeLeftBottom,
> 
>    EdkiiPlatformLogoDisplayAttributeCenterLeft,
> 
> -  EdkiiPlatformLogoDisplayAttributeCenter
> 
> +  EdkiiPlatformLogoDisplayAttributeCenter,
> 
> +  EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended
> 
>  } EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE;
> 
> 
> 
>  /**
> 
> diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> index 478ec2d40e..9065e5281b 100644
> --- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> +++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> @@ -169,7 +169,6 @@ BootLogoEnableLogo (
>          DestX = SizeOfX - Image.Width;
> 
>          DestY = 0;
> 
>          break;
> 
> -
> 
>        case EdkiiPlatformLogoDisplayAttributeCenterLeft:
> 
>          DestX = 0;
> 
>          DestY = (SizeOfY - Image.Height) / 2;
> 
> @@ -182,7 +181,6 @@ BootLogoEnableLogo (
>          DestX = SizeOfX - Image.Width;
> 
>          DestY = (SizeOfY - Image.Height) / 2;
> 
>          break;
> 
> -
> 
>        case EdkiiPlatformLogoDisplayAttributeLeftBottom:
> 
>          DestX = 0;
> 
>          DestY = SizeOfY - Image.Height;
> 
> @@ -195,7 +193,10 @@ BootLogoEnableLogo (
>          DestX = SizeOfX - Image.Width;
> 
>          DestY = SizeOfY - Image.Height;
> 
>          break;
> 
> -
> 
> +      case EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended:
> 
> +        DestX = (SizeOfX - Image.Width) / 2;
> 
> +        DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;
> 
> +        break;
> 
>        default:
> 
>          ASSERT (FALSE);
> 
>          continue;
> 
> --
> 2.34.1




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-09-26  8:09 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
  2022-10-08  1:37   ` 回复: " gaoliming
@ 2022-10-08  2:02   ` Ni, Ray
  2022-10-10  8:51     ` Sean Rhodes
  1 sibling, 1 reply; 30+ messages in thread
From: Ni, Ray @ 2022-10-08  2:02 UTC (permalink / raw)
  To: Rhodes, Sean, devel@edk2.groups.io
  Cc: Rhodes, Sean, Gao, Zhichao, Wang, Jian J, Gao, Liming

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-08  2:02   ` Ni, Ray
@ 2022-10-10  8:51     ` Sean Rhodes
  2022-10-10  9:25       ` Ni, Ray
  0 siblings, 1 reply; 30+ messages in thread
From: Sean Rhodes @ 2022-10-10  8:51 UTC (permalink / raw)
  To: Ni, Ray; +Cc: devel@edk2.groups.io, Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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: 9391 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-10  8:51     ` Sean Rhodes
@ 2022-10-10  9:25       ` Ni, Ray
  2022-10-10  9:44         ` Sean Rhodes
  2022-10-25  7:26         ` Sean Rhodes
  0 siblings, 2 replies; 30+ messages in thread
From: Ni, Ray @ 2022-10-10  9:25 UTC (permalink / raw)
  To: Rhodes, Sean
  Cc: devel@edk2.groups.io, Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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<mailto: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<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.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: [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<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      | 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: 13871 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-10  9:25       ` Ni, Ray
@ 2022-10-10  9:44         ` Sean Rhodes
  2022-10-25  7:26         ` Sean Rhodes
  1 sibling, 0 replies; 30+ messages in thread
From: Sean Rhodes @ 2022-10-10  9:44 UTC (permalink / raw)
  To: Ni, Ray
  Cc: devel@edk2.groups.io, Gao, Zhichao, Wang, Jian J, Gao, Liming,
	Matt DeVillier

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

+ Matt

@Matt DeVillier <matt.devillier@gmail.com> Does Ray's code work for you?


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: 12571 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-10  9:25       ` Ni, Ray
  2022-10-10  9:44         ` Sean Rhodes
@ 2022-10-25  7:26         ` Sean Rhodes
  2022-10-25  7:58           ` [edk2-devel] " Ni, Ray
  1 sibling, 1 reply; 30+ messages in thread
From: Sean Rhodes @ 2022-10-25  7:26 UTC (permalink / raw)
  To: Ni, Ray; +Cc: devel@edk2.groups.io, Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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: 12805 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-25  7:26         ` Sean Rhodes
@ 2022-10-25  7:58           ` Ni, Ray
  2022-10-25 16:20             ` Michael D Kinney
  0 siblings, 1 reply; 30+ messages in thread
From: Ni, Ray @ 2022-10-25  7:58 UTC (permalink / raw)
  To: devel@edk2.groups.io, Rhodes, Sean
  Cc: Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 18102 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-25  7:58           ` [edk2-devel] " Ni, Ray
@ 2022-10-25 16:20             ` Michael D Kinney
  2022-10-25 19:57               ` Sean Rhodes
                                 ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Michael D Kinney @ 2022-10-25 16:20 UTC (permalink / raw)
  To: devel@edk2.groups.io, Ni, Ray, Rhodes, Sean, Kinney, Michael D
  Cc: Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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<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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 56990 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-25 16:20             ` Michael D Kinney
@ 2022-10-25 19:57               ` Sean Rhodes
  2022-10-26  2:31               ` Ni, Ray
       [not found]               ` <17217DAE805D1AD6.492@groups.io>
  2 siblings, 0 replies; 30+ messages in thread
From: Sean Rhodes @ 2022-10-25 19:57 UTC (permalink / raw)
  To: Michael D Kinney, devel

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

Hi Mike/Ray

Thanks - so you mean something like https://github.com/tianocore/edk2/pull/3528? ( https://github.com/tianocore/edk2/pull/3528 ) (Just for example)

If not, I'm not sure how to control it without the PCD?

Sean

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-25 16:20             ` Michael D Kinney
  2022-10-25 19:57               ` Sean Rhodes
@ 2022-10-26  2:31               ` Ni, Ray
  2022-10-26  4:55                 ` Michael D Kinney
       [not found]               ` <17217DAE805D1AD6.492@groups.io>
  2 siblings, 1 reply; 30+ messages in thread
From: Ni, Ray @ 2022-10-26  2:31 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Rhodes, Sean
  Cc: Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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<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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 25330 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-26  2:31               ` Ni, Ray
@ 2022-10-26  4:55                 ` Michael D Kinney
  2022-10-26 20:30                   ` Sean Rhodes
  0 siblings, 1 reply; 30+ messages in thread
From: Michael D Kinney @ 2022-10-26  4:55 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Rhodes, Sean, Kinney, Michael D
  Cc: Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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@intel.com>
Sent: Tuesday, October 25, 2022 7:32 PM
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<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 64176 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-26  4:55                 ` Michael D Kinney
@ 2022-10-26 20:30                   ` Sean Rhodes
  2022-10-26 22:27                     ` Michael D Kinney
  0 siblings, 1 reply; 30+ messages in thread
From: Sean Rhodes @ 2022-10-26 20:30 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: Ni, Ray, devel@edk2.groups.io, Gao, Zhichao, Wang, Jian J,
	Gao, Liming

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

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@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@intel.com>
> *Sent:* Tuesday, October 25, 2022 7:32 PM
> *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: 21299 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2022-10-26 20:30                   ` Sean Rhodes
@ 2022-10-26 22:27                     ` Michael D Kinney
  0 siblings, 0 replies; 30+ messages in thread
From: Michael D Kinney @ 2022-10-26 22:27 UTC (permalink / raw)
  To: devel@edk2.groups.io, Rhodes, Sean, Kinney, Michael D
  Cc: Ni, Ray, Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes
Sent: Wednesday, October 26, 2022 1:30 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Ni, Ray <ray.ni@intel.com>; 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 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@intel.com<mailto:michael.d.kinney@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@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, October 25, 2022 7:32 PM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 71188 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
       [not found]               ` <17217DAE805D1AD6.492@groups.io>
@ 2023-03-08  9:01                 ` Ni, Ray
  2023-03-10 13:43                   ` Sean Rhodes
  0 siblings, 1 reply; 30+ messages in thread
From: Ni, Ray @ 2023-03-08  9:01 UTC (permalink / raw)
  To: devel@edk2.groups.io, Ni, Ray, Kinney, Michael D, Rhodes, Sean
  Cc: Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 27046 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-08  9:01                 ` Ni, Ray
@ 2023-03-10 13:43                   ` Sean Rhodes
  2023-03-13 11:49                     ` Sheng Lean Tan
  2023-03-15  9:24                     ` Ni, Ray
  0 siblings, 2 replies; 30+ messages in thread
From: Sean Rhodes @ 2023-03-10 13:43 UTC (permalink / raw)
  To: Ni, Ray
  Cc: devel@edk2.groups.io, Kinney, Michael D, Gao, Zhichao,
	Wang, Jian J, Gao, Liming

[-- 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 --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-10 13:43                   ` Sean Rhodes
@ 2023-03-13 11:49                     ` Sheng Lean Tan
  2023-03-15  8:53                       ` Sheng Lean Tan
  2023-03-15  9:24                     ` Ni, Ray
  1 sibling, 1 reply; 30+ messages in thread
From: Sheng Lean Tan @ 2023-03-13 11:49 UTC (permalink / raw)
  To: devel, sean
  Cc: Ni, Ray, Kinney, Michael D, Gao, Zhichao, Wang, Jian J,
	Gao, Liming

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

Hi Ray,
What is your thought on this?

Best Regards,
*Lean Sheng Tan*



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: sheng.tan@9elements.com
Phone: *+49 234 68 94 188 <+492346894188>*
Mobile: *+49 176 76 113842 <+4917676113842>*

Registered office: Bochum
Commercial register: Amtsgericht Bochum, HRB 17519
Management: Sebastian German, Eray Bazaar

Data protection information according to Art. 13 GDPR
<https://9elements.com/privacy>


On Fri, 10 Mar 2023 at 14:43, Sean Rhodes <sean@starlabs.systems> wrote:

> 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: 24601 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-13 11:49                     ` Sheng Lean Tan
@ 2023-03-15  8:53                       ` Sheng Lean Tan
  0 siblings, 0 replies; 30+ messages in thread
From: Sheng Lean Tan @ 2023-03-15  8:53 UTC (permalink / raw)
  To: devel, sean
  Cc: Ni, Ray, Kinney, Michael D, Gao, Zhichao, Wang, Jian J,
	Gao, Liming

[-- Attachment #1: Type: text/html, Size: 26303 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-10 13:43                   ` Sean Rhodes
  2023-03-13 11:49                     ` Sheng Lean Tan
@ 2023-03-15  9:24                     ` Ni, Ray
  2023-03-15 15:34                       ` Michael D Kinney
  1 sibling, 1 reply; 30+ messages in thread
From: Ni, Ray @ 2023-03-15  9:24 UTC (permalink / raw)
  To: devel@edk2.groups.io, Rhodes, Sean
  Cc: Kinney, Michael D, Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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 <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 <michael.d.kinney@intel.com>; 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

>                 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<mailto: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<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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 31316 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-15  9:24                     ` Ni, Ray
@ 2023-03-15 15:34                       ` Michael D Kinney
  2023-03-20  8:12                         ` Sheng Lean Tan
  0 siblings, 1 reply; 30+ messages in thread
From: Michael D Kinney @ 2023-03-15 15:34 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Rhodes, Sean
  Cc: Gao, Zhichao, Wang, Jian J, Gao, Liming, Kinney, Michael D

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

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.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, 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

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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto: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<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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 73493 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-15 15:34                       ` Michael D Kinney
@ 2023-03-20  8:12                         ` Sheng Lean Tan
  2023-03-20  9:56                           ` Ni, Ray
  0 siblings, 1 reply; 30+ messages in thread
From: Sheng Lean Tan @ 2023-03-20  8:12 UTC (permalink / raw)
  To: devel, Ni, Ray, michael.d.kinney
  Cc: Rhodes, Sean, Gao, Zhichao, Wang, Jian J, Gao, Liming

[-- Attachment #1: Type: text/html, Size: 67378 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-20  8:12                         ` Sheng Lean Tan
@ 2023-03-20  9:56                           ` Ni, Ray
  2023-03-20 12:46                             ` Sean Rhodes
  0 siblings, 1 reply; 30+ messages in thread
From: Ni, Ray @ 2023-03-20  9:56 UTC (permalink / raw)
  To: Tan, Lean Sheng, devel@edk2.groups.io, Kinney, Michael D
  Cc: Rhodes, Sean, Gao, Zhichao, Wang, Jian J, Gao, Liming

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

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.tan@9elements.com>
Sent: Monday, March 20, 2023 4:12 PM
To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Rhodes, Sean <sean@starlabs.systems>; 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,
Any feedback per Mic feedback?
On 15. Mar 2023, at 16:35, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@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.ni@intel.com<mailto:ray.ni@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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto: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<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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 40247 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-20  9:56                           ` Ni, Ray
@ 2023-03-20 12:46                             ` Sean Rhodes
  2023-03-21  7:19                               ` Ni, Ray
  0 siblings, 1 reply; 30+ messages in thread
From: Sean Rhodes @ 2023-03-20 12:46 UTC (permalink / raw)
  To: Ni, Ray
  Cc: Tan, Lean Sheng, devel@edk2.groups.io, Kinney, Michael D,
	Gao, Zhichao, Wang, Jian J, Gao, Liming

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

Hi Ray

Some members of the coreboot community want to centre the logo (as it is
currently) and some want to use the Microsoft recommended position. All we
want to do is have the option to build edk2 with one or the other.

Thanks

Sean

On Mon, 20 Mar 2023 at 09:56, Ni, Ray <ray.ni@intel.com> wrote:

> 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.tan@9elements.com>
> *Sent:* Monday, March 20, 2023 4:12 PM
> *To:* devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> *Cc:* Rhodes, Sean <sean@starlabs.systems>; 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,
>
> Any feedback per Mic feedback?
>
> Sent from my iPhone
>
>
> On 15. Mar 2023, at 16:35, Michael D Kinney <michael.d.kinney@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.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, 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
>
>
>
> 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 <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 <michael.d.kinney@intel.com>;
> 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
>
>
>
> >                 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?
>
>    1. 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: 31716 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo
  2023-03-20 12:46                             ` Sean Rhodes
@ 2023-03-21  7:19                               ` Ni, Ray
  0 siblings, 0 replies; 30+ messages in thread
From: Ni, Ray @ 2023-03-21  7:19 UTC (permalink / raw)
  To: Rhodes, Sean
  Cc: Tan, Lean Sheng, devel@edk2.groups.io, Kinney, Michael D,
	Gao, Zhichao, Wang, Jian J, Gao, Liming

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

Can the below solution work for you?

  1.  create a new Logo driver in UefiPayloadPkg
  2.  That logo driver defines a PCD such as MSPreferedLogoLocation, depending on its value, the driver returns the proper X/Y location.

I am trying to avoid changing the simple/sample one for a specific need.

Thanks,
Ray

From: Sean Rhodes <sean@starlabs.systems>
Sent: Monday, March 20, 2023 8:47 PM
To: Ni, Ray <ray.ni@intel.com>
Cc: Tan, Lean Sheng <sheng.tan@9elements.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; 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

Some members of the coreboot community want to centre the logo (as it is currently) and some want to use the Microsoft recommended position. All we want to do is have the option to build edk2 with one or the other.

Thanks

Sean

On Mon, 20 Mar 2023 at 09:56, Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> wrote:
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.tan@9elements.com<mailto:sheng.tan@9elements.com>>
Sent: Monday, March 20, 2023 4:12 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.kinney@intel.com<mailto:michael.d.kinney@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.ni@intel.com<mailto:ray.ni@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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto: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<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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, October 26, 2022 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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.gao@intel.com<mailto:zhichao.gao@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 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.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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 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<mailto: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<mailto:sean@starlabs.systems>>
Sent: Monday, October 10, 2022 4:51 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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: [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<mailto: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<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.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: [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<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      | 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: 44860 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2023-03-21  7:19 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-26  8:09 [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations Sean Rhodes
2022-09-26  8:09 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
2022-10-08  1:37   ` 回复: " gaoliming
2022-10-08  2:02   ` Ni, Ray
2022-10-10  8:51     ` Sean Rhodes
2022-10-10  9:25       ` Ni, Ray
2022-10-10  9:44         ` Sean Rhodes
2022-10-25  7:26         ` Sean Rhodes
2022-10-25  7:58           ` [edk2-devel] " Ni, Ray
2022-10-25 16:20             ` Michael D Kinney
2022-10-25 19:57               ` Sean Rhodes
2022-10-26  2:31               ` Ni, Ray
2022-10-26  4:55                 ` Michael D Kinney
2022-10-26 20:30                   ` Sean Rhodes
2022-10-26 22:27                     ` Michael D Kinney
     [not found]               ` <17217DAE805D1AD6.492@groups.io>
2023-03-08  9:01                 ` Ni, Ray
2023-03-10 13:43                   ` Sean Rhodes
2023-03-13 11:49                     ` Sheng Lean Tan
2023-03-15  8:53                       ` Sheng Lean Tan
2023-03-15  9:24                     ` Ni, Ray
2023-03-15 15:34                       ` Michael D Kinney
2023-03-20  8:12                         ` Sheng Lean Tan
2023-03-20  9:56                           ` Ni, Ray
2023-03-20 12:46                             ` Sean Rhodes
2023-03-21  7:19                               ` Ni, Ray
2022-09-26  8:09 ` [PATCH 3/3] UefiPayloadPkg: Hook up MICROSOFT_RECOMMENDED macro to PcdFollowMicrosoftRecommended Sean Rhodes
2022-10-03  9:22 ` [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow Microsoft Recommendations Sheng Lean Tan
     [not found] ` <171A84BCA8A12F72.28182@groups.io>
2022-10-07  9:14   ` Sheng Lean Tan
2022-10-08  1:37 ` 回复: " gaoliming
  -- strict thread matches above, loose matches on Subject: below --
2022-07-26  8:15 [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow BGRT specification Sean Rhodes
2022-07-26  8:15 ` [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo Sean Rhodes
2022-08-05  5:58   ` 回复: " gaoliming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox