1. Patchew
  2. EDK2
  3. [edk2] [PATCH edk2-platforms v2 00/18] ARM: Update GOP
  4. View patch

[edk2] [PATCH edk2-platforms v2 13/18] ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format

evan.lloyd@arm.com posted 18 patches 7 years, 4 months ago
[edk2] [PATCH edk2-platforms v2 13/18] ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format
Posted by evan.lloyd@arm.com 7 years, 4 months ago 
From: Girish Pathak <girish.pathak at arm.com>

Current HDLCD and PL111 platform libraries do not support display modes
with PixelBlueGreenRedReserved8BitPerColor format,  i.e. because of
historical confusion, they do not support the UEFI default
PixelBlueGreenRedReserved8BitPerColor

LcdPlatformLib for PL111, LcdPlatformQueryMode function returns the
pixel format as PixelRedGreenBlueReserved8BitPerColor which is wrong,
because that does not match the display controller's pixel format which
is set to BGR in PL111Lcd GOP driver.

Also it is not possible to configure pixel format as RGB/BGR for the
display modes for a platform at build time.

This change adds PcdGopPixelFormat to configure pixel format as
    PixelRedGreenBlueReserved8BitPerColor    or
    PixelBlueGreenRedReserved8BitPerColor    or
    PixelBitMask.
With this change, pixel format can be selected in the platform specific
.dsc file for all supported display modes.

Support for PixelBitMask is not implemented in PL111 or HDLCD
GOP driver, hence  HDLCD and PL111 platform libraries will return error
EFI_UNSUPPORTED if PcdGopPixelFormat is set to PixelBitMask.
Indeed, it is not clear what selecting PixelBitMask might mean, but
the option is allowed as it might suit a custom platform.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Girish Pathak <girish.pathak@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  1 +
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  1 +
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 23 ++++++++----
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 38 +++++++++-----------
 4 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index 2e83736609cf8c63cb498368cd377f6a23964ef4..4cbd324338be76a0b0bfb811159d893628e74155 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -41,3 +41,4 @@ [Protocols]
 
 [FixedPcd]
   gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId
+  gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index 1ee878041559fa84a810f65175f2507bda663d76..20045380149241ce14f41bcb70afcb8145fe821f 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
@@ -42,3 +42,4 @@ [Protocols]
 [FixedPcd]
   gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
   gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId
+  gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index b448d70f86d6acbc6bdae9749c7b6d0205935ba7..f1c18ac955f621e9eab3dede86758f5f1b1a791d 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -15,7 +15,6 @@
 #include <PiDxe.h>
 
 #include <Library/ArmPlatformSysConfigLib.h>
-#include <Library/IoLib.h>
 #include <Library/PcdLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DxeServicesTableLib.h>
@@ -93,6 +92,10 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
   @param[in] Handle              Handle to the LCD device instance.
 
   @retval EFI_SUCCESS            Plaform library initialized successfully.
+  @retval EFI_UNSUPPORTED        PcdGopPixelFormat must be
+                                 PixelRedGreenBlueReserved8BitPerColor OR
+                                 PixelBlueGreenRedReserved8BitPerColor
+                                 any other format is not supported.
   @retval !(EFI_SUCCESS)         Other errors.
 **/
 EFI_STATUS
@@ -101,6 +104,17 @@ LcdPlatformInitializeDisplay (
   )
 {
   EFI_STATUS  Status;
+  EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
+
+  // PixelBitMask and PixelBltOnly pixel formats are not supported
+  PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
+  if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
+    && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
+
+    ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
+      ||  PixelFormat == PixelBlueGreenRedReserved8BitPerColor);
+   return EFI_UNSUPPORTED;
+  }
 
   // Set the FPGA multiplexer to select the video output from the
   // motherboard or the daughterboard
@@ -285,12 +299,7 @@ LcdPlatformQueryMode (
   Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
   Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
 
-  /* Bits per Pixel is always LCD_BITS_PER_PIXEL_24 */
-  Info->PixelFormat                   = PixelRedGreenBlueReserved8BitPerColor;
-  Info->PixelInformation.RedMask      = LCD_24BPP_RED_MASK;
-  Info->PixelInformation.GreenMask    = LCD_24BPP_GREEN_MASK;
-  Info->PixelInformation.BlueMask     = LCD_24BPP_BLUE_MASK;
-  Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK;
+  Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 439cbdb1a73145fc4dc9c3c9587ce3fd9b9fff56..16a74c76cf27526493165dc6d81384f752fd3f20 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -149,7 +149,12 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
 /** PL111 Platform specific initialization function.
 
   @param[in] Handle              Handle to the LCD device instance.
+
   @retval EFI_SUCCESS            Plaform library initialized successfully.
+  @retval EFI_UNSUPPORTED        PcdGopPixelFormat must be
+                                 PixelRedGreenBlueReserved8BitPerColor OR
+                                 PixelBlueGreenRedReserved8BitPerColor
+                                 any other format is not supported
   @retval !(EFI_SUCCESS)         Other errors.
 **/
 EFI_STATUS
@@ -158,6 +163,17 @@ LcdPlatformInitializeDisplay (
   )
 {
   EFI_STATUS  Status;
+  EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
+
+  // PixelBitMask and PixelBltOnly pixel formats are not supported
+  PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
+  if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
+    && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
+
+    ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
+      ||  PixelFormat == PixelBlueGreenRedReserved8BitPerColor);
+   return EFI_UNSUPPORTED;
+  }
 
   // Set the FPGA multiplexer to select the video output from the motherboard
   // or the daughterboard
@@ -360,27 +376,7 @@ LcdPlatformQueryMode (
   Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
   Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
 
-  switch (mDisplayModes[ModeNumber].Bpp) {
-  case LCD_BITS_PER_PIXEL_24:
-    Info->PixelFormat                   = PixelRedGreenBlueReserved8BitPerColor;
-    Info->PixelInformation.RedMask      = LCD_24BPP_RED_MASK;
-    Info->PixelInformation.GreenMask    = LCD_24BPP_GREEN_MASK;
-    Info->PixelInformation.BlueMask     = LCD_24BPP_BLUE_MASK;
-    Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK;
-    break;
-
-  case LCD_BITS_PER_PIXEL_16_555:
-  case LCD_BITS_PER_PIXEL_16_565:
-  case LCD_BITS_PER_PIXEL_12_444:
-  case LCD_BITS_PER_PIXEL_8:
-  case LCD_BITS_PER_PIXEL_4:
-  case LCD_BITS_PER_PIXEL_2:
-  case LCD_BITS_PER_PIXEL_1:
-  default:
-    // These are not supported
-    ASSERT (FALSE);
-    break;
-  }
+  Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
 
   return EFI_SUCCESS;
 }
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v2 13/18] ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format
Posted by Ard Biesheuvel 7 years, 4 months ago 
On 22 December 2017 at 19:08,  <evan.lloyd@arm.com> wrote:
> From: Girish Pathak <girish.pathak at arm.com>
>
> Current HDLCD and PL111 platform libraries do not support display modes
> with PixelBlueGreenRedReserved8BitPerColor format,  i.e. because of
> historical confusion, they do not support the UEFI default
> PixelBlueGreenRedReserved8BitPerColor
>
> LcdPlatformLib for PL111, LcdPlatformQueryMode function returns the
> pixel format as PixelRedGreenBlueReserved8BitPerColor which is wrong,
> because that does not match the display controller's pixel format which
> is set to BGR in PL111Lcd GOP driver.
>
> Also it is not possible to configure pixel format as RGB/BGR for the
> display modes for a platform at build time.
>
> This change adds PcdGopPixelFormat to configure pixel format as
>     PixelRedGreenBlueReserved8BitPerColor    or
>     PixelBlueGreenRedReserved8BitPerColor    or
>     PixelBitMask.
> With this change, pixel format can be selected in the platform specific
> .dsc file for all supported display modes.
>
> Support for PixelBitMask is not implemented in PL111 or HDLCD
> GOP driver, hence  HDLCD and PL111 platform libraries will return error
> EFI_UNSUPPORTED if PcdGopPixelFormat is set to PixelBitMask.
> Indeed, it is not clear what selecting PixelBitMask might mean, but
> the option is allowed as it might suit a custom platform.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak <girish.pathak@arm.com>
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
> ---
>  Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  1 +
>  Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  1 +
>  Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 23 ++++++++----
>  Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 38 +++++++++-----------
>  4 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> index 2e83736609cf8c63cb498368cd377f6a23964ef4..4cbd324338be76a0b0bfb811159d893628e74155 100644
> --- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> +++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> @@ -41,3 +41,4 @@ [Protocols]
>
>  [FixedPcd]
>    gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId
> +  gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
> diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> index 1ee878041559fa84a810f65175f2507bda663d76..20045380149241ce14f41bcb70afcb8145fe821f 100644
> --- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> +++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> @@ -42,3 +42,4 @@ [Protocols]
>  [FixedPcd]
>    gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
>    gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId
> +  gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
> diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> index b448d70f86d6acbc6bdae9749c7b6d0205935ba7..f1c18ac955f621e9eab3dede86758f5f1b1a791d 100644
> --- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> +++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> @@ -15,7 +15,6 @@
>  #include <PiDxe.h>
>
>  #include <Library/ArmPlatformSysConfigLib.h>
> -#include <Library/IoLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/DxeServicesTableLib.h>
> @@ -93,6 +92,10 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
>    @param[in] Handle              Handle to the LCD device instance.
>
>    @retval EFI_SUCCESS            Plaform library initialized successfully.
> +  @retval EFI_UNSUPPORTED        PcdGopPixelFormat must be
> +                                 PixelRedGreenBlueReserved8BitPerColor OR
> +                                 PixelBlueGreenRedReserved8BitPerColor
> +                                 any other format is not supported.
>    @retval !(EFI_SUCCESS)         Other errors.
>  **/
>  EFI_STATUS
> @@ -101,6 +104,17 @@ LcdPlatformInitializeDisplay (
>    )
>  {
>    EFI_STATUS  Status;
> +  EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
> +
> +  // PixelBitMask and PixelBltOnly pixel formats are not supported
> +  PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
> +  if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
> +    && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
> +

Please fix the weird indentation here

> +    ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
> +      ||  PixelFormat == PixelBlueGreenRedReserved8BitPerColor);

and here (just put the boolean operator on the previous line)

> +   return EFI_UNSUPPORTED;
> +  }
>
>    // Set the FPGA multiplexer to select the video output from the
>    // motherboard or the daughterboard
> @@ -285,12 +299,7 @@ LcdPlatformQueryMode (
>    Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
>    Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
>
> -  /* Bits per Pixel is always LCD_BITS_PER_PIXEL_24 */
> -  Info->PixelFormat                   = PixelRedGreenBlueReserved8BitPerColor;
> -  Info->PixelInformation.RedMask      = LCD_24BPP_RED_MASK;
> -  Info->PixelInformation.GreenMask    = LCD_24BPP_GREEN_MASK;
> -  Info->PixelInformation.BlueMask     = LCD_24BPP_BLUE_MASK;
> -  Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK;
> +  Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
>
>    return EFI_SUCCESS;
>  }
> diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 439cbdb1a73145fc4dc9c3c9587ce3fd9b9fff56..16a74c76cf27526493165dc6d81384f752fd3f20 100644
> --- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -149,7 +149,12 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
>  /** PL111 Platform specific initialization function.
>
>    @param[in] Handle              Handle to the LCD device instance.
> +
>    @retval EFI_SUCCESS            Plaform library initialized successfully.
> +  @retval EFI_UNSUPPORTED        PcdGopPixelFormat must be
> +                                 PixelRedGreenBlueReserved8BitPerColor OR
> +                                 PixelBlueGreenRedReserved8BitPerColor
> +                                 any other format is not supported
>    @retval !(EFI_SUCCESS)         Other errors.
>  **/
>  EFI_STATUS
> @@ -158,6 +163,17 @@ LcdPlatformInitializeDisplay (
>    )
>  {
>    EFI_STATUS  Status;
> +  EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
> +
> +  // PixelBitMask and PixelBltOnly pixel formats are not supported
> +  PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
> +  if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
> +    && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
> +
> +    ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
> +      ||  PixelFormat == PixelBlueGreenRedReserved8BitPerColor);

Same here

> +   return EFI_UNSUPPORTED;
> +  }
>
>    // Set the FPGA multiplexer to select the video output from the motherboard
>    // or the daughterboard
> @@ -360,27 +376,7 @@ LcdPlatformQueryMode (
>    Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
>    Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
>
> -  switch (mDisplayModes[ModeNumber].Bpp) {
> -  case LCD_BITS_PER_PIXEL_24:
> -    Info->PixelFormat                   = PixelRedGreenBlueReserved8BitPerColor;
> -    Info->PixelInformation.RedMask      = LCD_24BPP_RED_MASK;
> -    Info->PixelInformation.GreenMask    = LCD_24BPP_GREEN_MASK;
> -    Info->PixelInformation.BlueMask     = LCD_24BPP_BLUE_MASK;
> -    Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK;
> -    break;
> -
> -  case LCD_BITS_PER_PIXEL_16_555:
> -  case LCD_BITS_PER_PIXEL_16_565:
> -  case LCD_BITS_PER_PIXEL_12_444:
> -  case LCD_BITS_PER_PIXEL_8:
> -  case LCD_BITS_PER_PIXEL_4:
> -  case LCD_BITS_PER_PIXEL_2:
> -  case LCD_BITS_PER_PIXEL_1:
> -  default:
> -    // These are not supported
> -    ASSERT (FALSE);
> -    break;
> -  }
> +  Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
>
>    return EFI_SUCCESS;
>  }
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>

With the above fixed

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.