From: Girish Pathak <girish.pathak@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!
In LcdPlatformLib for PL111, LcdPlatformQueryMode 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>
---
ArmPlatformPkg/ArmPlatformPkg.dec | 9 ++++-
ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf | 1 +
ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf | 1 +
ArmPlatformPkg/Include/Library/LcdPlatformLib.h | 6 +++-
ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c | 23 ++++++++----
ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 37 +++++++++-----------
ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c | 32 +++++++++++------
ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c | 16 ++++++++-
8 files changed, 83 insertions(+), 42 deletions(-)
diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index b8a6b131632dc6788b73a034a41b9e3176dffafa..548d2b211753275337c6973e05227cee8451c185 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -1,6 +1,6 @@
#/** @file
#
-# Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+# Copyright (c) 2011-2017, ARM Limited. All rights reserved.
# Copyright (c) 2015, Intel Corporation. All rights reserved.
#
# This program and the accompanying materials
@@ -126,6 +126,13 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L""|VOID*|0x0000001B
gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L""|VOID*|0x0000001C
+ # Graphics Output Pixel format
+ # 0 : PixelRedGreenBlueReserved8BitPerColor
+ # 1 : PixelBlueGreenRedReserved8BitPerColor
+ # 2 : PixelBitMask
+ # Default is set to UEFI console font format PixelBlueGreenRedReserved8BitPerColor
+ gArmPlatformTokenSpaceGuid.PcdGopPixelFormat|0x00000001|UINT32|0x00000040
+
[PcdsFixedAtBuild.common,PcdsDynamic.common]
## PL031 RealTimeClock
gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x00000024
diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index bba851c9bd6089cee748b45f40599b24c1293785..37756481596c7e978ed9ed0a932eeb2aa0a3b657 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -42,3 +42,4 @@ [Protocols]
[FixedPcd]
gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId
+ gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index 1a044baf4698aa6bfa5cd6038f01e84f7a633ea9..6f1cb3b55ff814d007718b5597f821dd20100477 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
@@ -42,3 +42,4 @@ [Protocols]
[FixedPcd]
gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId
+ gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
index f2f345b18fd15f2cde159fd42d3208a28f598a1f..d357c22c46b62966859793372c447883e12e1e80 100644
--- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
+++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
@@ -139,7 +139,6 @@
#define LCD_12BPP_444_BLUE_MASK 0x0000000F
#define LCD_12BPP_444_RESERVED_MASK 0x0000F000
-
// The enumeration indexes maps the PL111 LcdBpp values used in the LCD Control Register
typedef enum {
LCD_BITS_PER_PIXEL_1 = 0,
@@ -165,6 +164,10 @@ typedef struct {
* @param IN Handle Handle to the LCD device instance.
*
* @retval EFI_SUCCESS Platform initialization success.
+ * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be
+ * PixelRedGreenBlueReserved8BitPerColor OR
+ * PixelBlueGreenRedReserved8BitPerColor
+ * any other format is not supported.
* @retval !(EFI_SUCCESS) Other errors.
**/
EFI_STATUS
@@ -216,6 +219,7 @@ LcdPlatformSetMode (
* (on success).
*
* @retval EFI_SUCCESS Success if the requested mode is found.
+ * @retval EFI_INVALID_PARAMETER Info is NULL.
* @retval EFI_INVALID_PARAMETER Requested mode not found.
**/
EFI_STATUS
diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index 2401cdb30cb7252964ce1f363aa26d99933c09be..07730ed0d7a84b3fb64047bd1013fbc97301db53 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/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>
@@ -91,6 +90,10 @@ EFI_EDID_ACTIVE_PROTOCOL mEdidActive = {
/** HDLCD Platform specific initialization function.
*
* @retval EFI_SUCCESS Plaform library initialization success.
+ * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be
+ * PixelRedGreenBlueReserved8BitPerColor OR
+ * PixelBlueGreenRedReserved8BitPerColor
+ * any other format is not supported.
* @retval !(EFI_SUCCESS) Other errors.
**/
EFI_STATUS
@@ -99,6 +102,7 @@ LcdPlatformInitializeDisplay (
)
{
EFI_STATUS Status;
+ EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
/* Set the FPGA multiplexer to select the video output from the
* motherboard or the daughterboard */
@@ -120,6 +124,16 @@ LcdPlatformInitializeDisplay (
NULL
);
+ // PixelBitMask and PixelBltOnly pixel formats are not supported
+ PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
+ if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
+ && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
+
+ ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
+ || PixelFormat == PixelBlueGreenRedReserved8BitPerColor);
+ return EFI_UNSUPPORTED;
+ }
+
return Status;
}
@@ -282,12 +296,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/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 753c5b615361f83625cdd4f0506909721da014b6..31d5e037e42a51f1c08e5bf2fa22eeb7be23e45f 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -149,6 +149,10 @@ EFI_EDID_ACTIVE_PROTOCOL mEdidActive = {
/** PL111 Platform specific initialization function.
*
* @retval EFI_SUCCESS Plaform library initialization success.
+ * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be
+ * PixelRedGreenBlueReserved8BitPerColor OR
+ * PixelBlueGreenRedReserved8BitPerColor
+ * any other format is not supported
* @retval !(EFI_SUCCESS) Other errors.
**/
EFI_STATUS
@@ -157,6 +161,7 @@ LcdPlatformInitializeDisplay (
)
{
EFI_STATUS Status;
+ EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
// Set the FPGA multiplexer to select the video output from the motherboard or the daughterboard
Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, PL111_CLCD_SITE);
@@ -172,6 +177,16 @@ LcdPlatformInitializeDisplay (
);
}
+ // PixelBitMask and PixelBltOnly pixel formats are not supported
+ PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
+ if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
+ && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
+
+ ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
+ || PixelFormat == PixelBlueGreenRedReserved8BitPerColor);
+ return EFI_UNSUPPORTED;
+ }
+
return Status;
}
@@ -366,27 +381,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;
}
diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
index 03153c06d314cb497c91889386ca6075c0c9f718..3ea7feca439e7ae9a610b6b3139ddbfad3ac6f9c 100644
--- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
+++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
@@ -22,6 +22,8 @@
#include "LcdGraphicsOutputDxe.h"
+#define BYTES_PER_PIXEL 4
+
/**********************************************************************
*
* This file contains all the bits of the Lcd that are
@@ -66,10 +68,6 @@ LcdInitialize (
HDLCD_LITTLE_ENDIAN | HDLCD_4BYTES_PER_PIXEL
);
- MmioWrite32 (HDLCD_REG_RED_SELECT, (0 << 16 | 8 << 8 | 0));
- MmioWrite32 (HDLCD_REG_GREEN_SELECT, (0 << 16 | 8 << 8 | 8));
- MmioWrite32 (HDLCD_REG_BLUE_SELECT, (0 << 16 | 8 << 8 | 16));
-
return EFI_SUCCESS;
}
@@ -77,7 +75,8 @@ LcdInitialize (
*
* @param ModeNumber Display mode number.
* @retval EFI_SUCCESS Display set mode success.
- * @retval EFI_DEVICE_ERROR If mode not found/supported.
+ * @retval EFI_DEVICE_ERROR Mode not found/supported.
+ * @retval !EFI_SUCCESS Other errors.
**/
EFI_STATUS
LcdSetMode (
@@ -87,9 +86,8 @@ LcdSetMode (
EFI_STATUS Status;
CONST SCAN_TIMINGS *Horizontal;
CONST SCAN_TIMINGS *Vertical;
- UINT32 BytesPerPixel;
- LCD_BPP LcdBpp;
+ EFI_GRAPHICS_OUTPUT_MODE_INFORMATION ModeInfo;
// Set the video mode timings and other relevant information
Status = LcdPlatformGetTimings (
@@ -104,13 +102,22 @@ LcdSetMode (
ASSERT (Horizontal != NULL);
ASSERT (Vertical != NULL);
- Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
+ // Get the pixel format information.
+ Status = LcdPlatformQueryMode (ModeNumber, &ModeInfo);
if (EFI_ERROR (Status)) {
ASSERT_EFI_ERROR (Status);
return Status;
}
- BytesPerPixel = GetBytesPerPixel (LcdBpp);
+ if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
+ MmioWrite32 (HDLCD_REG_RED_SELECT, (8 << 8 | 16));
+ MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8 | 0));
+ } else {
+ MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8 | 16));
+ MmioWrite32 (HDLCD_REG_RED_SELECT, (8 << 8 | 0));
+ }
+
+ MmioWrite32 (HDLCD_REG_GREEN_SELECT, (8 << 8 | 8));
// Disable the controller
MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
@@ -118,10 +125,13 @@ LcdSetMode (
// Update the frame buffer information with the new settings
MmioWrite32 (
HDLCD_REG_FB_LINE_LENGTH,
- Horizontal->Resolution * BytesPerPixel
+ Horizontal->Resolution * BYTES_PER_PIXEL
);
- MmioWrite32 (HDLCD_REG_FB_LINE_PITCH, Horizontal->Resolution * BytesPerPixel);
+ MmioWrite32 (
+ HDLCD_REG_FB_LINE_PITCH,
+ Horizontal->Resolution * BYTES_PER_PIXEL
+ );
MmioWrite32 (HDLCD_REG_FB_LINE_COUNT, Vertical->Resolution - 1);
diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
index 6de60491e9fd0c5bca71e743aac2862ff85f6e7e..4bad2367982e16d5d23c4eab2e6d91bf7db1c031 100644
--- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
+++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
@@ -80,6 +80,7 @@ LcdInitialize (
* @param ModeNumber Display mode number.
* @retval EFI_SUCCESS Display set mode success.
* @retval EFI_DEVICE_ERROR If mode not found/supported.
+ @retval !EFI_SUCCESS Other errors.
**/
EFI_STATUS
LcdSetMode (
@@ -92,6 +93,8 @@ LcdSetMode (
UINT32 LcdControl;
LCD_BPP LcdBpp;
+ EFI_GRAPHICS_OUTPUT_MODE_INFORMATION ModeInfo;
+
// Set the video mode timings and other relevant information
Status = LcdPlatformGetTimings (
ModeNumber,
@@ -111,6 +114,13 @@ LcdSetMode (
return Status;
}
+ // Get the pixel format information.
+ Status = LcdPlatformQueryMode (ModeNumber, &ModeInfo);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
// Disable the CLCD_LcdEn bit
MmioAnd32 (PL111_REG_LCD_CONTROL, ~PL111_CTRL_LCD_EN);
@@ -144,7 +154,11 @@ LcdSetMode (
// PL111_REG_LCD_CONTROL
LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP (LcdBpp)
- | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
+ | PL111_CTRL_LCD_TFT;
+
+ if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
+ LcdControl |= PL111_CTRL_BGR;
+ }
MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Sep 26, 2017 at 09:15:24PM +0100, evan.lloyd@arm.com wrote:
> From: Girish Pathak <girish.pathak@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!
>
> In LcdPlatformLib for PL111, LcdPlatformQueryMode 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>
> ---
> ArmPlatformPkg/ArmPlatformPkg.dec | 9 ++++-
> ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf | 1 +
> ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf | 1 +
> ArmPlatformPkg/Include/Library/LcdPlatformLib.h | 6 +++-
> ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c | 23 ++++++++----
> ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 37 +++++++++-----------
> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c | 32 +++++++++++------
> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c | 16 ++++++++-
> 8 files changed, 83 insertions(+), 42 deletions(-)
>
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index b8a6b131632dc6788b73a034a41b9e3176dffafa..548d2b211753275337c6973e05227cee8451c185 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -1,6 +1,6 @@
> #/** @file
> #
> -# Copyright (c) 2011-2016, ARM Limited. All rights reserved.
> +# Copyright (c) 2011-2017, ARM Limited. All rights reserved.
> # Copyright (c) 2015, Intel Corporation. All rights reserved.
> #
> # This program and the accompanying materials
> @@ -126,6 +126,13 @@ [PcdsFixedAtBuild.common]
> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L""|VOID*|0x0000001B
> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L""|VOID*|0x0000001C
>
> + # Graphics Output Pixel format
> + # 0 : PixelRedGreenBlueReserved8BitPerColor
> + # 1 : PixelBlueGreenRedReserved8BitPerColor
> + # 2 : PixelBitMask
> + # Default is set to UEFI console font format PixelBlueGreenRedReserved8BitPerColor
> + gArmPlatformTokenSpaceGuid.PcdGopPixelFormat|0x00000001|UINT32|0x00000040
> +
> [PcdsFixedAtBuild.common,PcdsDynamic.common]
> ## PL031 RealTimeClock
> gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x00000024
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> index bba851c9bd6089cee748b45f40599b24c1293785..37756481596c7e978ed9ed0a932eeb2aa0a3b657 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> @@ -42,3 +42,4 @@ [Protocols]
> [FixedPcd]
> gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
> gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId
> + gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> index 1a044baf4698aa6bfa5cd6038f01e84f7a633ea9..6f1cb3b55ff814d007718b5597f821dd20100477 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> @@ -42,3 +42,4 @@ [Protocols]
> [FixedPcd]
> gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
> gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId
> + gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
> diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> index f2f345b18fd15f2cde159fd42d3208a28f598a1f..d357c22c46b62966859793372c447883e12e1e80 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -139,7 +139,6 @@
> #define LCD_12BPP_444_BLUE_MASK 0x0000000F
> #define LCD_12BPP_444_RESERVED_MASK 0x0000F000
>
> -
Spurious whitespace change adding an extra hunk to the patch, please drop.
> // The enumeration indexes maps the PL111 LcdBpp values used in the LCD Control Register
> typedef enum {
> LCD_BITS_PER_PIXEL_1 = 0,
> @@ -165,6 +164,10 @@ typedef struct {
> * @param IN Handle Handle to the LCD device instance.
> *
> * @retval EFI_SUCCESS Platform initialization success.
> + * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be
> + * PixelRedGreenBlueReserved8BitPerColor OR
> + * PixelBlueGreenRedReserved8BitPerColor
> + * any other format is not supported.
> * @retval !(EFI_SUCCESS) Other errors.
> **/
> EFI_STATUS
> @@ -216,6 +219,7 @@ LcdPlatformSetMode (
> * (on success).
> *
> * @retval EFI_SUCCESS Success if the requested mode is found.
> + * @retval EFI_INVALID_PARAMETER Info is NULL.
> * @retval EFI_INVALID_PARAMETER Requested mode not found.
> **/
> EFI_STATUS
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> index 2401cdb30cb7252964ce1f363aa26d99933c09be..07730ed0d7a84b3fb64047bd1013fbc97301db53 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/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>
> @@ -91,6 +90,10 @@ EFI_EDID_ACTIVE_PROTOCOL mEdidActive = {
> /** HDLCD Platform specific initialization function.
> *
> * @retval EFI_SUCCESS Plaform library initialization success.
> + * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be
> + * PixelRedGreenBlueReserved8BitPerColor OR
> + * PixelBlueGreenRedReserved8BitPerColor
> + * any other format is not supported.
> * @retval !(EFI_SUCCESS) Other errors.
> **/
> EFI_STATUS
> @@ -99,6 +102,7 @@ LcdPlatformInitializeDisplay (
> )
> {
> EFI_STATUS Status;
> + EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
>
> /* Set the FPGA multiplexer to select the video output from the
> * motherboard or the daughterboard */
> @@ -120,6 +124,16 @@ LcdPlatformInitializeDisplay (
> NULL
> );
>
> + // PixelBitMask and PixelBltOnly pixel formats are not supported
> + PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
> + if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
> + && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
> +
> + ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
> + || PixelFormat == PixelBlueGreenRedReserved8BitPerColor);
> + return EFI_UNSUPPORTED;
> + }
> +
This block looks identical to a similar block in the next file.
Could this be a shared helper function?
> return Status;
> }
>
> @@ -282,12 +296,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/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 753c5b615361f83625cdd4f0506909721da014b6..31d5e037e42a51f1c08e5bf2fa22eeb7be23e45f 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -149,6 +149,10 @@ EFI_EDID_ACTIVE_PROTOCOL mEdidActive = {
> /** PL111 Platform specific initialization function.
> *
> * @retval EFI_SUCCESS Plaform library initialization success.
> + * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be
> + * PixelRedGreenBlueReserved8BitPerColor OR
> + * PixelBlueGreenRedReserved8BitPerColor
> + * any other format is not supported
> * @retval !(EFI_SUCCESS) Other errors.
> **/
> EFI_STATUS
> @@ -157,6 +161,7 @@ LcdPlatformInitializeDisplay (
> )
> {
> EFI_STATUS Status;
> + EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
>
> // Set the FPGA multiplexer to select the video output from the motherboard or the daughterboard
> Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, PL111_CLCD_SITE);
> @@ -172,6 +177,16 @@ LcdPlatformInitializeDisplay (
> );
> }
>
> + // PixelBitMask and PixelBltOnly pixel formats are not supported
> + PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
> + if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
> + && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
> +
> + ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
> + || PixelFormat == PixelBlueGreenRedReserved8BitPerColor);
> + return EFI_UNSUPPORTED;
> + }
> +
(This one.)
> return Status;
> }
>
> @@ -366,27 +381,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;
> }
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> index 03153c06d314cb497c91889386ca6075c0c9f718..3ea7feca439e7ae9a610b6b3139ddbfad3ac6f9c 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> @@ -22,6 +22,8 @@
>
> #include "LcdGraphicsOutputDxe.h"
>
> +#define BYTES_PER_PIXEL 4
> +
> /**********************************************************************
> *
> * This file contains all the bits of the Lcd that are
> @@ -66,10 +68,6 @@ LcdInitialize (
> HDLCD_LITTLE_ENDIAN | HDLCD_4BYTES_PER_PIXEL
> );
>
> - MmioWrite32 (HDLCD_REG_RED_SELECT, (0 << 16 | 8 << 8 | 0));
> - MmioWrite32 (HDLCD_REG_GREEN_SELECT, (0 << 16 | 8 << 8 | 8));
> - MmioWrite32 (HDLCD_REG_BLUE_SELECT, (0 << 16 | 8 << 8 | 16));
> -
> return EFI_SUCCESS;
> }
>
> @@ -77,7 +75,8 @@ LcdInitialize (
> *
> * @param ModeNumber Display mode number.
> * @retval EFI_SUCCESS Display set mode success.
> - * @retval EFI_DEVICE_ERROR If mode not found/supported.
> + * @retval EFI_DEVICE_ERROR Mode not found/supported.
> + * @retval !EFI_SUCCESS Other errors.
> **/
> EFI_STATUS
> LcdSetMode (
> @@ -87,9 +86,8 @@ LcdSetMode (
> EFI_STATUS Status;
> CONST SCAN_TIMINGS *Horizontal;
> CONST SCAN_TIMINGS *Vertical;
> - UINT32 BytesPerPixel;
> - LCD_BPP LcdBpp;
>
> + EFI_GRAPHICS_OUTPUT_MODE_INFORMATION ModeInfo;
>
> // Set the video mode timings and other relevant information
> Status = LcdPlatformGetTimings (
> @@ -104,13 +102,22 @@ LcdSetMode (
> ASSERT (Horizontal != NULL);
> ASSERT (Vertical != NULL);
>
> - Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
> + // Get the pixel format information.
> + Status = LcdPlatformQueryMode (ModeNumber, &ModeInfo);
> if (EFI_ERROR (Status)) {
> ASSERT_EFI_ERROR (Status);
> return Status;
> }
>
> - BytesPerPixel = GetBytesPerPixel (LcdBpp);
> + if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
> + MmioWrite32 (HDLCD_REG_RED_SELECT, (8 << 8 | 16));
> + MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8 | 0));
> + } else {
> + MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8 | 16));
> + MmioWrite32 (HDLCD_REG_RED_SELECT, (8 << 8 | 0));
> + }
> +
> + MmioWrite32 (HDLCD_REG_GREEN_SELECT, (8 << 8 | 8));
Could we have some parentheses above, to clarify (intended) precedence?
>
> // Disable the controller
> MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
> @@ -118,10 +125,13 @@ LcdSetMode (
> // Update the frame buffer information with the new settings
> MmioWrite32 (
> HDLCD_REG_FB_LINE_LENGTH,
> - Horizontal->Resolution * BytesPerPixel
> + Horizontal->Resolution * BYTES_PER_PIXEL
> );
>
> - MmioWrite32 (HDLCD_REG_FB_LINE_PITCH, Horizontal->Resolution * BytesPerPixel);
> + MmioWrite32 (
> + HDLCD_REG_FB_LINE_PITCH,
> + Horizontal->Resolution * BYTES_PER_PIXEL
> + );
>
> MmioWrite32 (HDLCD_REG_FB_LINE_COUNT, Vertical->Resolution - 1);
>
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> index 6de60491e9fd0c5bca71e743aac2862ff85f6e7e..4bad2367982e16d5d23c4eab2e6d91bf7db1c031 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> @@ -80,6 +80,7 @@ LcdInitialize (
> * @param ModeNumber Display mode number.
> * @retval EFI_SUCCESS Display set mode success.
> * @retval EFI_DEVICE_ERROR If mode not found/supported.
> + @retval !EFI_SUCCESS Other errors.
> **/
> EFI_STATUS
> LcdSetMode (
> @@ -92,6 +93,8 @@ LcdSetMode (
> UINT32 LcdControl;
> LCD_BPP LcdBpp;
>
> + EFI_GRAPHICS_OUTPUT_MODE_INFORMATION ModeInfo;
> +
> // Set the video mode timings and other relevant information
> Status = LcdPlatformGetTimings (
> ModeNumber,
> @@ -111,6 +114,13 @@ LcdSetMode (
> return Status;
> }
>
> + // Get the pixel format information.
> + Status = LcdPlatformQueryMode (ModeNumber, &ModeInfo);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return Status;
> + }
> +
> // Disable the CLCD_LcdEn bit
> MmioAnd32 (PL111_REG_LCD_CONTROL, ~PL111_CTRL_LCD_EN);
>
> @@ -144,7 +154,11 @@ LcdSetMode (
>
> // PL111_REG_LCD_CONTROL
> LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP (LcdBpp)
> - | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
> + | PL111_CTRL_LCD_TFT;
This is not a function call, so I do not believe the continuation line
should be indented.
/
Leif
> +
> + if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
> + LcdControl |= PL111_CTRL_BGR;
> + }
>
> MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
>
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.