public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms v3 00/17] Update GOP
@ 2018-03-20 16:18 Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 01/17] ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries Girish Pathak
                   ` (17 more replies)
  0 siblings, 18 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

This patch series addresses comments on the patch v2
(https://lists.01.org/pipermail/edk2-devel/2017-December/019406.html)
reworking of the Graphics Output Protocol code in ArmPlatformPkg.
It also contains updates for the new SCMI protocol (MTL Library).

Code is available for examination at:
  https://github.com/girishpathak/edk2-platforms/tree/201_gop_v3


Ard Biesheuvel (1):
  ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries

EvanLloyd (2):
  ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp
  ARM/VExpressPkg: Redefine LcdPlatformGetTimings function

Girish Pathak (14):
  ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding standard
  ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments
  ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD inf
  ARM/VExpressPkg: Add and update debug ASSERTS
  ARM/VExpressPkg: PL111Lcd/HdLcd plaform libs: Minor code cleanup
  ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32
  ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check EFI_TIMEOUT
  ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format
  ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
  ARM/VExpressPkg: Reserving framebuffer at build
  ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer
  ARM/VExpressPkg: New DP500/DP550/DP650 platform library
  ARM/JunoPkg: Adding SCMI MTL library
  ARM/JunoPkg: Add HDLCD platform library

 Platform/ARM/JunoPkg/ArmJuno.dec                                                   |  17 +-
 Platform/ARM/JunoPkg/ArmJuno.dsc                                                   |  31 +-
 Platform/ARM/JunoPkg/ArmJuno.fdf                                                   |  12 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf                             |   5 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c                               |  29 +-
 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c                         | 198 +++++++
 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf                       |  39 ++
 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h                  |  94 ++++
 Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c                        | 555 ++++++++++++++++++++
 Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf                   |  41 ++
 Platform/ARM/VExpressPkg/ArmVExpressPkg.dec                                        |   3 +-
 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c                       | 387 ++++++++++++++
 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf                     |  43 ++
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf             |   7 +-
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c                      |  53 +-
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 310 +++++++----
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  14 +-
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 389 +++++++++-----
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  10 +-
 19 files changed, 1945 insertions(+), 292 deletions(-)
 create mode 100644 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c
 create mode 100644 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
 create mode 100644 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h
 create mode 100644 Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
 create mode 100644 Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
 create mode 100644 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
 create mode 100644 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 01/17] ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 02/17] ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding standard Girish Pathak
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

This change fixes incorrect MODULE_TYPE of HDLCD and PL111
LcdPlatformLibs. Currently set MODUL_TYPE DXE_DRIVER is incorrect
for these platform libraries. Hence set this to type BASE.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
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       | 4 ++--
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index fc51c781b45122eaf4f2269af61b44c8630cdfb8..5bbfb7eabb5d475a8c6ef21d0a3f75490be47467 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -2,7 +2,7 @@
 #
 #  Component description file for HdLcdArmLib module
 #
-#  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -18,7 +18,7 @@ [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = HdLcdArmVExpress
   FILE_GUID                      = 535a720e-06c0-4bb9-b563-452216abbed4
-  MODULE_TYPE                    = DXE_DRIVER
+  MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = LcdPlatformLib
 
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index fd83d2412d4fd2dfa59204f21626c62e377e3c55..7ffd217a7d1eefb06b99042c2b2a9ed0079f2bd3 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
@@ -2,7 +2,7 @@
 #
 #  Component description file for ArmVeGraphicsDxe module
 #
-#  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -18,7 +18,7 @@ [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = PL111LcdArmVExpressLib
   FILE_GUID                      = b7f06f20-496f-11e0-a8e8-0002a5d5c51b
-  MODULE_TYPE                    = DXE_DRIVER
+  MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = LcdPlatformLib
 
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 02/17] ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding standard
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 01/17] ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 03/17] ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments Girish Pathak
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

There is no functional modification in this change
As preparation for further work, the formatting is corrected to meet
the EDKII coding standard.

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c       | 136 ++++++-----
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf  |   5 +-
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 238 +++++++++++---------
 3 files changed, 218 insertions(+), 161 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index b1106ee19b98cebac01820924514eac79b97d0d5..36ea484bbceac51566bfeaf029b1aa0ede93dee1 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -1,6 +1,6 @@
-/**
+/** @file
 
-  Copyright (c) 2012, ARM Ltd. All rights reserved.
+  Copyright (c) 2012-2018, ARM Ltd. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -44,35 +44,40 @@ typedef struct {
   UINT32                     VFrontPorch;
 } LCD_RESOLUTION;
 
-
 LCD_RESOLUTION mResolutions[] = {
   { // Mode 0 : VGA : 640 x 480 x 24 bpp
-    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, VGA_OSC_FREQUENCY,
+    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    VGA_OSC_FREQUENCY,
     VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
     VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
   },
   { // Mode 1 : SVGA : 800 x 600 x 24 bpp
-    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, SVGA_OSC_FREQUENCY,
+    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    SVGA_OSC_FREQUENCY,
     SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
     SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
   },
   { // Mode 2 : XGA : 1024 x 768 x 24 bpp
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, XGA_OSC_FREQUENCY,
+    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    XGA_OSC_FREQUENCY,
     XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
     XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
   },
   { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
-    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (SXGA_OSC_FREQUENCY/2),
+    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    (SXGA_OSC_FREQUENCY/2),
     SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
     SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
   },
   { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
-    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (UXGA_OSC_FREQUENCY/2),
+    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    (UXGA_OSC_FREQUENCY/2),
     UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
     UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
   },
   { // Mode 5 : HD : 1920 x 1080 x 24 bpp
-    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (HD_OSC_FREQUENCY/2),
+    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    (HD_OSC_FREQUENCY/2),
     HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
     HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
   }
@@ -95,19 +100,25 @@ LcdPlatformInitializeDisplay (
 {
   EFI_STATUS  Status;
 
-  // Set the FPGA multiplexer to select the video output from the motherboard or the daughterboard
-  Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, ARM_VE_DAUGHTERBOARD_1_SITE);
-  if (EFI_ERROR(Status)) {
+  // Set the FPGA multiplexer to select the video output from the
+  // motherboard or the daughterboard
+  Status = ArmPlatformSysConfigSet (
+             SYS_CFG_MUXFPGA,
+             ARM_VE_DAUGHTERBOARD_1_SITE
+             );
+  if (EFI_ERROR (Status)) {
     return Status;
   }
 
   // Install the EDID Protocols
   Status = gBS->InstallMultipleProtocolInterfaces (
-    &Handle,
-    &gEfiEdidDiscoveredProtocolGuid,  &mEdidDiscovered,
-    &gEfiEdidActiveProtocolGuid,      &mEdidActive,
-    NULL
-  );
+                  &Handle,
+                  &gEfiEdidDiscoveredProtocolGuid,
+                  &mEdidDiscovered,
+                  &gEfiEdidActiveProtocolGuid,
+                  &mEdidActive,
+                  NULL
+                  );
 
   return Status;
 }
@@ -132,16 +143,25 @@ LcdPlatformGetVram (
   } else {
     AllocationType = AllocateAddress;
   }
-  Status = gBS->AllocatePages (AllocationType, EfiBootServicesData, EFI_SIZE_TO_PAGES(((UINTN)LCD_VRAM_SIZE)), VramBaseAddress);
-  if (EFI_ERROR(Status)) {
+  Status = gBS->AllocatePages (
+                  AllocationType,
+                  EfiBootServicesData,
+                  EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
+                  VramBaseAddress
+                  );
+  if (EFI_ERROR (Status)) {
     return Status;
   }
 
-  // Mark the VRAM as write-combining. The VRAM is inside the DRAM, which is cacheable.
-  Status = gDS->SetMemorySpaceAttributes (*VramBaseAddress, *VramSize,
-                  EFI_MEMORY_WC);
-  ASSERT_EFI_ERROR(Status);
-  if (EFI_ERROR(Status)) {
+  // Mark the VRAM as write-combining.
+  // The VRAM is inside the DRAM, which is cacheable.
+  Status = gDS->SetMemorySpaceAttributes (
+                  *VramBaseAddress,
+                  *VramSize,
+                  EFI_MEMORY_WC
+                  );
+  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
     gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
     return Status;
   }
@@ -150,15 +170,11 @@ LcdPlatformGetVram (
 }
 
 UINT32
-LcdPlatformGetMaxMode (
-  VOID
-  )
+LcdPlatformGetMaxMode (VOID)
 {
-  //
   // The following line will report correctly the total number of graphics modes
-  // that could be supported by the graphics driver:
-  //
-  return (sizeof(mResolutions) / sizeof(LCD_RESOLUTION));
+  // that could be supported by the graphics driver
+  return (sizeof (mResolutions) / sizeof (LCD_RESOLUTION));
 }
 
 EFI_STATUS
@@ -174,25 +190,35 @@ LcdPlatformSetMode (
 
   // Set the video mode oscillator
   do {
-    Status = ArmPlatformSysConfigSetDevice (SYS_CFG_OSC_SITE1, PcdGet32(PcdHdLcdVideoModeOscId), mResolutions[ModeNumber].OscFreq);
+    Status = ArmPlatformSysConfigSetDevice (
+               SYS_CFG_OSC_SITE1,
+               PcdGet32 (PcdHdLcdVideoModeOscId),
+               mResolutions[ModeNumber].OscFreq
+               );
   } while (Status == EFI_TIMEOUT);
-  if (EFI_ERROR(Status)) {
+  if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     return Status;
   }
 
   // Set the DVI into the new mode
   do {
-    Status = ArmPlatformSysConfigSet (SYS_CFG_DVIMODE, mResolutions[ModeNumber].Mode);
+    Status = ArmPlatformSysConfigSet (
+               SYS_CFG_DVIMODE,
+               mResolutions[ModeNumber].Mode
+               );
   } while (Status == EFI_TIMEOUT);
-  if (EFI_ERROR(Status)) {
+  if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     return Status;
   }
 
   // Set the multiplexer
-  Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, ARM_VE_DAUGHTERBOARD_1_SITE);
-  if (EFI_ERROR(Status)) {
+  Status = ArmPlatformSysConfigSet (
+             SYS_CFG_MUXFPGA,
+             ARM_VE_DAUGHTERBOARD_1_SITE
+             );
+  if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     return Status;
   }
@@ -216,25 +242,25 @@ LcdPlatformQueryMode (
   Info->PixelsPerScanLine = mResolutions[ModeNumber].HorizontalResolution;
 
   switch (mResolutions[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_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;
+  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;
   }
 
   return EFI_SUCCESS;
diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index 5bbfb7eabb5d475a8c6ef21d0a3f75490be47467..bedbaea3a296bea5184564c582659381733d08a9 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -1,6 +1,6 @@
 #/** @file
 #
-#  Component description file for HdLcdArmLib module
+#  Component description file for HdLcdArmVExpress module
 #
 #  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.<BR>
 #
@@ -23,8 +23,7 @@ [Defines]
   LIBRARY_CLASS                  = LcdPlatformLib
 
 [Sources.common]
-
-HdLcdArmVExpress.c
+  HdLcdArmVExpress.c
 
 [Packages]
   ArmPlatformPkg/ArmPlatformPkg.dec
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 18b7ef5cf57f1de8dbd9068731eb21d3f76cbf06..3ab9fe4abb15a41731a518a8500cd414f670fd66 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2011-2015, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -41,87 +41,102 @@ typedef struct {
   UINT32                     VFrontPorch;
 } LCD_RESOLUTION;
 
-
 LCD_RESOLUTION mResolutions[] = {
   { // Mode 0 : VGA : 640 x 480 x 24 bpp
-      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, VGA_OSC_FREQUENCY,
-      VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-      VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    VGA_OSC_FREQUENCY,
+    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
+    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
   },
   { // Mode 1 : SVGA : 800 x 600 x 24 bpp
-      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, SVGA_OSC_FREQUENCY,
-      SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-      SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    SVGA_OSC_FREQUENCY,
+    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
+    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
   },
   { // Mode 2 : XGA : 1024 x 768 x 24 bpp
-      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, XGA_OSC_FREQUENCY,
-      XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-      XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    XGA_OSC_FREQUENCY,
+    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
+    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
   },
   { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
-      SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (SXGA_OSC_FREQUENCY/2),
-      SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
-      SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
+    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    (SXGA_OSC_FREQUENCY/2),
+    SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
+    SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
   },
   { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
-      UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (UXGA_OSC_FREQUENCY/2),
-      UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
-      UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
+    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    (UXGA_OSC_FREQUENCY/2),
+    UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
+    UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
   },
   { // Mode 5 : HD : 1920 x 1080 x 24 bpp
-      HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (HD_OSC_FREQUENCY/2),
-      HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
-      HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
+    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    (HD_OSC_FREQUENCY/2),
+    HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
+    HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
   },
   { // Mode 6 : VGA : 640 x 480 x 16 bpp (565 Mode)
-      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565, VGA_OSC_FREQUENCY,
-      VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-      VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
+    VGA_OSC_FREQUENCY,
+    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
+    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
   },
   { // Mode 7 : SVGA : 800 x 600 x 16 bpp (565 Mode)
-      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565, SVGA_OSC_FREQUENCY,
-      SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-      SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
+    SVGA_OSC_FREQUENCY,
+    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
+    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
   },
   { // Mode 8 : XGA : 1024 x 768 x 16 bpp (565 Mode)
-      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565, XGA_OSC_FREQUENCY,
-      XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-      XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
+    XGA_OSC_FREQUENCY,
+    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
+    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
   },
   { // Mode 9 : VGA : 640 x 480 x 15 bpp
-      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, VGA_OSC_FREQUENCY,
-      VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-      VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    VGA_OSC_FREQUENCY,
+    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
+    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
   },
   { // Mode 10 : SVGA : 800 x 600 x 15 bpp
-      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, SVGA_OSC_FREQUENCY,
-      SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-      SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    SVGA_OSC_FREQUENCY,
+    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
+    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
   },
   { // Mode 11 : XGA : 1024 x 768 x 15 bpp
-      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, XGA_OSC_FREQUENCY,
-      XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-      XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    XGA_OSC_FREQUENCY,
+    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
+    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
   },
   { // Mode 12 : XGA : 1024 x 768 x 15 bpp - All the timing info is derived from Linux Kernel Driver Settings
-      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, 63500000,
-      XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-      XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    63500000,
+    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
+    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
   },
   { // Mode 13 : VGA : 640 x 480 x 12 bpp (444 Mode)
-      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444, VGA_OSC_FREQUENCY,
-      VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-      VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
+    VGA_OSC_FREQUENCY,
+    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
+    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
   },
   { // Mode 14 : SVGA : 800 x 600 x 12 bpp (444 Mode)
-      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444, SVGA_OSC_FREQUENCY,
-      SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-      SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
+    SVGA_OSC_FREQUENCY,
+    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
+    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
   },
   { // Mode 15 : XGA : 1024 x 768 x 12 bpp (444 Mode)
-      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444, XGA_OSC_FREQUENCY,
-      XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-      XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
+    XGA_OSC_FREQUENCY,
+    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
+    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
   }
 };
 
@@ -135,7 +150,6 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
   NULL
 };
 
-
 EFI_STATUS
 LcdPlatformInitializeDisplay (
   IN EFI_HANDLE   Handle
@@ -143,16 +157,19 @@ LcdPlatformInitializeDisplay (
 {
   EFI_STATUS  Status;
 
-  // Set the FPGA multiplexer to select the video output from the motherboard or the daughterboard
+  // Set the FPGA multiplexer to select the video output from the motherboard
+  // or the daughterboard
   Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, PL111_CLCD_SITE);
-  if (!EFI_ERROR(Status)) {
+  if (!EFI_ERROR (Status)) {
     // Install the EDID Protocols
-    Status = gBS->InstallMultipleProtocolInterfaces(
-      &Handle,
-      &gEfiEdidDiscoveredProtocolGuid,  &mEdidDiscovered,
-      &gEfiEdidActiveProtocolGuid,      &mEdidActive,
-      NULL
-    );
+    Status = gBS->InstallMultipleProtocolInterfaces (
+                    &Handle,
+                    &gEfiEdidDiscoveredProtocolGuid,
+                    &mEdidDiscovered,
+                    &gEfiEdidActiveProtocolGuid,
+                    &mEdidActive,
+                    NULL
+                    );
   }
 
   return Status;
@@ -169,29 +186,38 @@ LcdPlatformGetVram (
   Status = EFI_SUCCESS;
 
   // Is it on the motherboard or on the daughterboard?
-  switch(PL111_CLCD_SITE) {
+  switch (PL111_CLCD_SITE) {
 
   case ARM_VE_MOTHERBOARD_SITE:
-    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS) PL111_CLCD_VRAM_MOTHERBOARD_BASE;
+    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS)PL111_CLCD_VRAM_MOTHERBOARD_BASE;
     *VramSize = LCD_VRAM_SIZE;
     break;
 
   case ARM_VE_DAUGHTERBOARD_1_SITE:
-    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS) LCD_VRAM_CORE_TILE_BASE;
+    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS)LCD_VRAM_CORE_TILE_BASE;
     *VramSize = LCD_VRAM_SIZE;
 
     // Allocate the VRAM from the DRAM so that nobody else uses it.
-    Status = gBS->AllocatePages( AllocateAddress, EfiBootServicesData, EFI_SIZE_TO_PAGES(((UINTN)LCD_VRAM_SIZE)), VramBaseAddress);
-    if (EFI_ERROR(Status)) {
+    Status = gBS->AllocatePages (
+                    AllocateAddress,
+                    EfiBootServicesData,
+                    EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
+                    VramBaseAddress
+                    );
+    if (EFI_ERROR (Status)) {
       return Status;
     }
 
-    // Mark the VRAM as write-combining. The VRAM is inside the DRAM, which is cacheable.
-    Status = gDS->SetMemorySpaceAttributes (*VramBaseAddress, *VramSize,
-                    EFI_MEMORY_WC);
-    ASSERT_EFI_ERROR(Status);
-    if (EFI_ERROR(Status)) {
-      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES(*VramSize));
+    // Mark the VRAM as write-combining.
+    // The VRAM is inside the DRAM, which is cacheable.
+    Status = gDS->SetMemorySpaceAttributes (
+                    *VramBaseAddress,
+                    *VramSize,
+                    EFI_MEMORY_WC
+                    );
+    ASSERT_EFI_ERROR (Status);
+    if (EFI_ERROR (Status)) {
+      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
       return Status;
     }
     break;
@@ -206,19 +232,18 @@ LcdPlatformGetVram (
 }
 
 UINT32
-LcdPlatformGetMaxMode (
-  VOID
-  )
+LcdPlatformGetMaxMode (VOID)
 {
-  // The following line will report correctly the total number of graphics modes
-  // supported by the PL111CLCD.
-  //return (sizeof(mResolutions) / sizeof(CLCD_RESOLUTION)) - 1;
+  // The following line would correctly report the total number
+  // of graphics modes supported by the PL111CLCD.
+  // return (sizeof(mResolutions) / sizeof(CLCD_RESOLUTION)) - 1;
 
   // However, on some platforms it is desirable to ignore some graphics modes.
-  // This could be because the specific implementation of PL111 has certain limitations.
+  // This could be because the specific implementation of PL111 has
+  // certain limitations.
 
   // Set the maximum mode allowed
-  return (PcdGet32(PcdPL111LcdMaxMode));
+  return (PcdGet32 (PcdPL111LcdMaxMode));
 }
 
 EFI_STATUS
@@ -238,22 +263,26 @@ LcdPlatformSetMode (
 
   LcdSite = PL111_CLCD_SITE;
 
-  switch(LcdSite) {
+  switch (LcdSite) {
   case ARM_VE_MOTHERBOARD_SITE:
     Function = SYS_CFG_OSC;
     OscillatorId = PL111_CLCD_MOTHERBOARD_VIDEO_MODE_OSC_ID;
     break;
   case ARM_VE_DAUGHTERBOARD_1_SITE:
     Function = SYS_CFG_OSC_SITE1;
-    OscillatorId = (UINT32)PcdGet32(PcdPL111LcdVideoModeOscId);
+    OscillatorId = (UINT32)PcdGet32 (PcdPL111LcdVideoModeOscId);
     break;
   default:
     return EFI_UNSUPPORTED;
   }
 
   // Set the video mode oscillator
-  Status = ArmPlatformSysConfigSetDevice (Function, OscillatorId, mResolutions[ModeNumber].OscFreq);
-  if (EFI_ERROR(Status)) {
+  Status = ArmPlatformSysConfigSetDevice (
+             Function,
+             OscillatorId,
+             mResolutions[ModeNumber].OscFreq
+             );
+  if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     return Status;
   }
@@ -267,8 +296,11 @@ LcdPlatformSetMode (
     SysId &= ~(ARM_FVP_SYS_ID_VARIANT_MASK | ARM_FVP_SYS_ID_REV_MASK);
     if (SysId != ARM_FVP_BASE_BOARD_SYS_ID) {
       // Set the DVI into the new mode
-      Status = ArmPlatformSysConfigSet (SYS_CFG_DVIMODE, mResolutions[ModeNumber].Mode);
-      if (EFI_ERROR(Status)) {
+      Status = ArmPlatformSysConfigSet (
+                 SYS_CFG_DVIMODE,
+                 mResolutions[ModeNumber].Mode
+                 );
+      if (EFI_ERROR (Status)) {
         ASSERT_EFI_ERROR (Status);
         return Status;
       }
@@ -277,7 +309,7 @@ LcdPlatformSetMode (
 
   // Set the multiplexer
   Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, LcdSite);
-  if (EFI_ERROR(Status)) {
+  if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     return Status;
   }
@@ -301,25 +333,25 @@ LcdPlatformQueryMode (
   Info->PixelsPerScanLine = mResolutions[ModeNumber].HorizontalResolution;
 
   switch (mResolutions[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_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;
+  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;
   }
 
   return EFI_SUCCESS;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 03/17] ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 01/17] ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 02/17] ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding standard Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 04/17] ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD inf Girish Pathak
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

There is no functional modification in this change.
In this change some comments in HDLCD and PL111LCD platform library
code are modified and a few new comments are added. This is to
prevent mixing formatting changes with functional changes.

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 74 ++++++++++++++++++++
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 73 +++++++++++++++++++
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  2 +-
 3 files changed, 148 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index 36ea484bbceac51566bfeaf029b1aa0ede93dee1..80603f04df3793b8b62196990c846de9ba8f130d 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -44,6 +44,8 @@ typedef struct {
   UINT32                     VFrontPorch;
 } LCD_RESOLUTION;
 
+/** The display modes supported by the platform.
+**/
 LCD_RESOLUTION mResolutions[] = {
   { // Mode 0 : VGA : 640 x 480 x 24 bpp
     VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
@@ -93,6 +95,13 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
   NULL
 };
 
+/** HDLCD platform specific initialization function.
+
+  @param[in] Handle              Handle to the LCD device instance.
+
+  @retval EFI_SUCCESS            Plaform library initialized successfully.
+  @retval !(EFI_SUCCESS)         Other errors.
+**/
 EFI_STATUS
 LcdPlatformInitializeDisplay (
   IN EFI_HANDLE   Handle
@@ -123,6 +132,18 @@ LcdPlatformInitializeDisplay (
   return Status;
 }
 
+/** Allocate VRAM memory in DRAM for the framebuffer
+  (unless it is reserved already).
+
+  The allocated address can be used to set the framebuffer.
+
+  @param[out] VramBaseAddress     A pointer to the framebuffer address.
+  @param[out] VramSize            A pointer to the size of the framebuffer
+                                  in bytes
+
+  @retval EFI_SUCCESS             Framebuffer memory allocated successfully.
+  @retval !(EFI_SUCCESS)          Other errors.
+**/
 EFI_STATUS
 LcdPlatformGetVram (
   OUT EFI_PHYSICAL_ADDRESS*  VramBaseAddress,
@@ -169,6 +190,13 @@ LcdPlatformGetVram (
   return EFI_SUCCESS;
 }
 
+/** Return total number of modes supported.
+
+  Note: Valid mode numbers are 0 to MaxMode - 1
+  See Section 12.9 of the UEFI Specification 2.7
+
+  @retval UINT32             Mode Number.
+**/
 UINT32
 LcdPlatformGetMaxMode (VOID)
 {
@@ -177,6 +205,14 @@ LcdPlatformGetMaxMode (VOID)
   return (sizeof (mResolutions) / sizeof (LCD_RESOLUTION));
 }
 
+/** Set the requested display mode.
+
+  @param[in] ModeNumber             Mode Number.
+
+  @retval EFI_SUCCESS               Mode set successfully.
+  @retval EFI_INVALID_PARAMETER     Requested mode not found.
+  @retval !(EFI_SUCCESS)            Other errors.
+**/
 EFI_STATUS
 LcdPlatformSetMode (
   IN UINT32                         ModeNumber
@@ -226,6 +262,17 @@ LcdPlatformSetMode (
   return Status;
 }
 
+/** Return information for the requested mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] Info                Pointer for returned mode information
+                                  (on success).
+
+  @retval EFI_SUCCESS             Mode information for the requested mode
+                                  returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
 EFI_STATUS
 LcdPlatformQueryMode (
   IN  UINT32                                ModeNumber,
@@ -266,6 +313,23 @@ LcdPlatformQueryMode (
   return EFI_SUCCESS;
 }
 
+/** Return display timing information for the requested mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] HRes                Pointer to horizontal resolution.
+  @param[out] HSync               Pointer to horizontal sync width.
+  @param[out] HBackPorch          Pointer to horizontal back porch.
+  @param[out] HFrontPorch         Pointer to horizontal front porch.
+  @param[out] VRes                Pointer to vertical resolution.
+  @param[out] VSync               Pointer to vertical sync width.
+  @param[out] VBackPorch          Pointer to vertical back porch.
+  @param[out] VFrontPorch         Pointer to vertical front porch.
+
+  @retval EFI_SUCCESS             Display timing information for the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
 EFI_STATUS
 LcdPlatformGetTimings (
   IN  UINT32                              ModeNumber,
@@ -295,6 +359,16 @@ LcdPlatformGetTimings (
   return EFI_SUCCESS;
 }
 
+/** Return bits per pixel information for a mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] Bpp                 Pointer to bits per pixel information.
+
+  @retval EFI_SUCCESS             Bits per pixel information for the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
 EFI_STATUS
 LcdPlatformGetBpp (
   IN  UINT32                              ModeNumber,
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 3ab9fe4abb15a41731a518a8500cd414f670fd66..3e3102623ebc46cbe31b7f3500021f53f2281d1b 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -41,6 +41,8 @@ typedef struct {
   UINT32                     VFrontPorch;
 } LCD_RESOLUTION;
 
+/** The display modes supported by the platform.
+**/
 LCD_RESOLUTION mResolutions[] = {
   { // Mode 0 : VGA : 640 x 480 x 24 bpp
     VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
@@ -150,6 +152,12 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
   NULL
 };
 
+/** PL111 Platform specific initialization function.
+
+  @param[in] Handle              Handle to the LCD device instance.
+  @retval EFI_SUCCESS            Plaform library initialized successfully.
+  @retval !(EFI_SUCCESS)         Other errors.
+**/
 EFI_STATUS
 LcdPlatformInitializeDisplay (
   IN EFI_HANDLE   Handle
@@ -175,6 +183,18 @@ LcdPlatformInitializeDisplay (
   return Status;
 }
 
+/** Allocate VRAM memory in DRAM for the framebuffer
+  (unless it is reserved already).
+
+  The allocated address can be used to set the framebuffer.
+
+  @param[out] VramBaseAddress     A pointer to the framebuffer address.
+  @param[out] VramSize            A pointer to the size of the framebuffer
+                                  in bytes
+
+  @retval EFI_SUCCESS             Framebuffer memory allocated successfully.
+  @retval !(EFI_SUCCESS)          Other errors.
+**/
 EFI_STATUS
 LcdPlatformGetVram (
   OUT EFI_PHYSICAL_ADDRESS*  VramBaseAddress,
@@ -231,6 +251,13 @@ LcdPlatformGetVram (
   return Status;
 }
 
+/** Return total number of modes supported.
+
+  Note: Valid mode numbers are 0 to MaxMode - 1
+  See Section 12.9 of the UEFI Specification 2.7
+
+  @retval UINT32             Mode Number.
+**/
 UINT32
 LcdPlatformGetMaxMode (VOID)
 {
@@ -246,6 +273,15 @@ LcdPlatformGetMaxMode (VOID)
   return (PcdGet32 (PcdPL111LcdMaxMode));
 }
 
+/** Set the requested display mode.
+
+  @param[in] ModeNumber            Mode Number.
+
+  @retval  EFI_SUCCESS             Mode set successfully.
+  @retval  EFI_INVALID_PARAMETER   Requested mode not found.
+  @retval  EFI_UNSUPPORTED         PLL111 configuration not supported.
+  @retval  !(EFI_SUCCESS)          Other errors.
+**/
 EFI_STATUS
 LcdPlatformSetMode (
   IN UINT32                         ModeNumber
@@ -317,6 +353,16 @@ LcdPlatformSetMode (
   return Status;
 }
 
+/** Return information for the requested mode number.
+
+  @param[in]  ModeNumber         Mode Number.
+  @param[out] Info               Pointer for returned mode information
+                                 (on success).
+
+  @retval EFI_SUCCESS             Mode information for the requested mode
+                                  returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
 EFI_STATUS
 LcdPlatformQueryMode (
   IN  UINT32                                ModeNumber,
@@ -357,6 +403,23 @@ LcdPlatformQueryMode (
   return EFI_SUCCESS;
 }
 
+/** Return display timing information for the requested mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] HRes                Pointer to horizontal resolution.
+  @param[out] HSync               Pointer to horizontal sync width.
+  @param[out] HBackPorch          Pointer to horizontal back porch.
+  @param[out] HFrontPorch         Pointer to horizontal front porch.
+  @param[out] VRes                Pointer to vertical resolution.
+  @param[out] VSync               Pointer to vertical sync width.
+  @param[out] VBackPorch          Pointer to vertical back porch.
+  @param[out] VFrontPorch         Pointer to vertical front porch.
+
+  @retval EFI_SUCCESS             Display timing information for the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
 EFI_STATUS
 LcdPlatformGetTimings (
   IN  UINT32                              ModeNumber,
@@ -386,6 +449,16 @@ LcdPlatformGetTimings (
   return EFI_SUCCESS;
 }
 
+/** Return bits per pixel information for a mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] Bpp                 Pointer to bits per pixel information.
+
+  @retval EFI_SUCCESS             Bits per pixel information for the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
 EFI_STATUS
 LcdPlatformGetBpp (
   IN  UINT32                              ModeNumber,
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index 7ffd217a7d1eefb06b99042c2b2a9ed0079f2bd3..9ca2ace9594d21050c3e53705054c25c69e238f4 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
@@ -1,6 +1,6 @@
 #/** @file
 #
-#  Component description file for ArmVeGraphicsDxe module
+#  Component description file for PL111LcdArmVExpressLib module
 #
 #  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.<BR>
 #
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 04/17] ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD inf
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (2 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 03/17] ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 05/17] ARM/VExpressPkg: Add and update debug ASSERTS Girish Pathak
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

PCD PcdPL111LcdMaxMode is not used in HDLCD platform library.
Presence of this PCD in HDLCD is probably due to copy/paste code
from PL111 Lcd platform library. This change removes it from
the HdLcdArmVExpressLib.inf file.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Girish Pathak <girish.pathak@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index bedbaea3a296bea5184564c582659381733d08a9..34db24d4ee6162b0e963d500a8ed1097bd8a5ceb 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -40,5 +40,4 @@ [Protocols]
   gEfiEdidActiveProtocolGuid                    # Produced
 
 [Pcd]
-  gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
   gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 05/17] ARM/VExpressPkg: Add and update debug ASSERTS
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (3 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 04/17] ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD inf Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 06/17] ARM/VExpressPkg: PL111Lcd/HdLcd plaform libs: Minor code cleanup Girish Pathak
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

This change adds some debug assertions e.g to catch NULL pointer errors
missing in PL11Lcd and HdLcd platform libraries.

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

Notes:
    V3:
    - Use ASSERT_EFI_ERROR (Status) in place of ASSERT (FALSE)   [Ard]
    
      Done                                                       [Girish]

 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c       | 22 +++++++++++++++++-
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 24 +++++++++++++++++++-
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index 80603f04df3793b8b62196990c846de9ba8f130d..5247b8e038ac45ba800d0f6c79f8718c99b951da 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -153,6 +153,9 @@ LcdPlatformGetVram (
   EFI_STATUS              Status;
   EFI_ALLOCATE_TYPE       AllocationType;
 
+  ASSERT (VramBaseAddress != NULL);
+  ASSERT (VramSize != NULL);
+
   // Set the vram size
   *VramSize = LCD_VRAM_SIZE;
 
@@ -171,6 +174,7 @@ LcdPlatformGetVram (
                   VramBaseAddress
                   );
   if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
     return Status;
   }
 
@@ -181,8 +185,8 @@ LcdPlatformGetVram (
                   *VramSize,
                   EFI_MEMORY_WC
                   );
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
     gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
     return Status;
   }
@@ -221,6 +225,7 @@ LcdPlatformSetMode (
   EFI_STATUS            Status;
 
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -279,7 +284,10 @@ LcdPlatformQueryMode (
   OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info
   )
 {
+  ASSERT (Info != NULL);
+
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -343,7 +351,18 @@ LcdPlatformGetTimings (
   OUT UINT32*                             VFrontPorch
   )
 {
+  // One of the pointers is NULL
+  ASSERT (HRes != NULL);
+  ASSERT (HSync != NULL);
+  ASSERT (HBackPorch != NULL);
+  ASSERT (HFrontPorch != NULL);
+  ASSERT (VRes != NULL);
+  ASSERT (VSync != NULL);
+  ASSERT (VBackPorch != NULL);
+  ASSERT (VFrontPorch != NULL);
+
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -376,6 +395,7 @@ LcdPlatformGetBpp (
   )
 {
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 3e3102623ebc46cbe31b7f3500021f53f2281d1b..3d2ec7b1bf658a5fd644cd82fd1341245ba0f5d3 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -205,6 +205,9 @@ LcdPlatformGetVram (
 
   Status = EFI_SUCCESS;
 
+  ASSERT (VramBaseAddress != NULL);
+  ASSERT (VramSize != NULL);
+
   // Is it on the motherboard or on the daughterboard?
   switch (PL111_CLCD_SITE) {
 
@@ -225,6 +228,7 @@ LcdPlatformGetVram (
                     VramBaseAddress
                     );
     if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
       return Status;
     }
 
@@ -235,8 +239,8 @@ LcdPlatformGetVram (
                     *VramSize,
                     EFI_MEMORY_WC
                     );
-    ASSERT_EFI_ERROR (Status);
     if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
       gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
       return Status;
     }
@@ -294,6 +298,7 @@ LcdPlatformSetMode (
   UINT32                SysId;
 
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -369,7 +374,10 @@ LcdPlatformQueryMode (
   OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info
   )
 {
+  ASSERT (Info != NULL);
+
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -433,7 +441,18 @@ LcdPlatformGetTimings (
   OUT UINT32*                             VFrontPorch
   )
 {
+  // One of the pointers is NULL
+  ASSERT (HRes != NULL);
+  ASSERT (HSync != NULL);
+  ASSERT (HBackPorch != NULL);
+  ASSERT (HFrontPorch != NULL);
+  ASSERT (VRes != NULL);
+  ASSERT (VSync != NULL);
+  ASSERT (VBackPorch != NULL);
+  ASSERT (VFrontPorch != NULL);
+
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -465,7 +484,10 @@ LcdPlatformGetBpp (
   OUT LCD_BPP  *                          Bpp
   )
 {
+  ASSERT (Bpp != NULL);
+
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 06/17] ARM/VExpressPkg: PL111Lcd/HdLcd plaform libs: Minor code cleanup
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (4 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 05/17] ARM/VExpressPkg: Add and update debug ASSERTS Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 07/17] ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32 Girish Pathak
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

This minor change removes some unecessary initializations and variables
in PL111LcdArmVExpress.c and redudant return status checks in
HdLcdArmVExpress.c

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c       |  5 +----
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 16 ++++------------
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index 5247b8e038ac45ba800d0f6c79f8718c99b951da..6fe7360d55c85ab9137b245b2fa7abf661de270e 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -259,10 +259,7 @@ LcdPlatformSetMode (
              SYS_CFG_MUXFPGA,
              ARM_VE_DAUGHTERBOARD_1_SITE
              );
-  if (EFI_ERROR (Status)) {
-    ASSERT_EFI_ERROR (Status);
-    return Status;
-  }
+  ASSERT_EFI_ERROR (Status);
 
   return Status;
 }
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 3d2ec7b1bf658a5fd644cd82fd1341245ba0f5d3..c229530ce8e7156d0b834abbe461a4d3213afc79 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -203,8 +203,6 @@ LcdPlatformGetVram (
 {
   EFI_STATUS              Status;
 
-  Status = EFI_SUCCESS;
-
   ASSERT (VramBaseAddress != NULL);
   ASSERT (VramSize != NULL);
 
@@ -214,6 +212,7 @@ LcdPlatformGetVram (
   case ARM_VE_MOTHERBOARD_SITE:
     *VramBaseAddress = (EFI_PHYSICAL_ADDRESS)PL111_CLCD_VRAM_MOTHERBOARD_BASE;
     *VramSize = LCD_VRAM_SIZE;
+    Status = EFI_SUCCESS;
     break;
 
   case ARM_VE_DAUGHTERBOARD_1_SITE:
@@ -242,7 +241,6 @@ LcdPlatformGetVram (
     if (EFI_ERROR (Status)) {
       ASSERT_EFI_ERROR (Status);
       gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
-      return Status;
     }
     break;
 
@@ -292,7 +290,6 @@ LcdPlatformSetMode (
   )
 {
   EFI_STATUS            Status;
-  UINT32                LcdSite;
   UINT32                OscillatorId;
   SYS_CONFIG_FUNCTION   Function;
   UINT32                SysId;
@@ -302,9 +299,7 @@ LcdPlatformSetMode (
     return EFI_INVALID_PARAMETER;
   }
 
-  LcdSite = PL111_CLCD_SITE;
-
-  switch (LcdSite) {
+  switch (PL111_CLCD_SITE) {
   case ARM_VE_MOTHERBOARD_SITE:
     Function = SYS_CFG_OSC;
     OscillatorId = PL111_CLCD_MOTHERBOARD_VIDEO_MODE_OSC_ID;
@@ -349,11 +344,8 @@ LcdPlatformSetMode (
   }
 
   // Set the multiplexer
-  Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, LcdSite);
-  if (EFI_ERROR (Status)) {
-    ASSERT_EFI_ERROR (Status);
-    return Status;
-  }
+  Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, PL111_CLCD_SITE);
+  ASSERT_EFI_ERROR (Status);
 
   return Status;
 }
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 07/17] ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (5 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 06/17] ARM/VExpressPkg: PL111Lcd/HdLcd plaform libs: Minor code cleanup Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 08/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check EFI_TIMEOUT Girish Pathak
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

This change replaces PcdGet32 with FixedPcdGet32 for the PCDs which
are defined as fixed PCDs.

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 2 +-
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       | 2 +-
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 4 ++--
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index 6fe7360d55c85ab9137b245b2fa7abf661de270e..d6d63295688f9aa5a6b24cee142760609439941d 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -233,7 +233,7 @@ LcdPlatformSetMode (
   do {
     Status = ArmPlatformSysConfigSetDevice (
                SYS_CFG_OSC_SITE1,
-               PcdGet32 (PcdHdLcdVideoModeOscId),
+               FixedPcdGet32 (PcdHdLcdVideoModeOscId),
                mResolutions[ModeNumber].OscFreq
                );
   } while (Status == EFI_TIMEOUT);
diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index 34db24d4ee6162b0e963d500a8ed1097bd8a5ceb..eb2a3c94b80129a16bf9ae26b7c2a5403556dc71 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -39,5 +39,5 @@ [Protocols]
   gEfiEdidDiscoveredProtocolGuid                # Produced
   gEfiEdidActiveProtocolGuid                    # Produced
 
-[Pcd]
+[FixedPcd]
   gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index c229530ce8e7156d0b834abbe461a4d3213afc79..c51540e1c00a1f46ca845d95306e28e7ca6bfde5 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -272,7 +272,7 @@ LcdPlatformGetMaxMode (VOID)
   // certain limitations.
 
   // Set the maximum mode allowed
-  return (PcdGet32 (PcdPL111LcdMaxMode));
+  return (FixedPcdGet32 (PcdPL111LcdMaxMode));
 }
 
 /** Set the requested display mode.
@@ -306,7 +306,7 @@ LcdPlatformSetMode (
     break;
   case ARM_VE_DAUGHTERBOARD_1_SITE:
     Function = SYS_CFG_OSC_SITE1;
-    OscillatorId = (UINT32)PcdGet32 (PcdPL111LcdVideoModeOscId);
+    OscillatorId = FixedPcdGet32 (PcdPL111LcdVideoModeOscId);
     break;
   default:
     return EFI_UNSUPPORTED;
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index 9ca2ace9594d21050c3e53705054c25c69e238f4..f480000936f394accc98e8d031fcd2900a6f4611 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
@@ -39,6 +39,6 @@ [Protocols]
   gEfiEdidDiscoveredProtocolGuid                # Produced
   gEfiEdidActiveProtocolGuid                    # Produced
 
-[Pcd]
+[FixedPcd]
   gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode
   gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 08/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check EFI_TIMEOUT
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (6 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 07/17] ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32 Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 09/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp Girish Pathak
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

None of the ArmPlatformSys*  functions returns EFI_TIMEOUT. Hence checking
this in the do {} while loop in LcdPlatformSetMode is wrong. Therefore
remove this comparision and as a result remove the do {} while loop.

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c | 22 ++++++++------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index d6d63295688f9aa5a6b24cee142760609439941d..cd62cd61883dcd78c72aa614466c6f714cc73fc4 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -230,25 +230,21 @@ LcdPlatformSetMode (
   }
 
   // Set the video mode oscillator
-  do {
-    Status = ArmPlatformSysConfigSetDevice (
-               SYS_CFG_OSC_SITE1,
-               FixedPcdGet32 (PcdHdLcdVideoModeOscId),
-               mResolutions[ModeNumber].OscFreq
-               );
-  } while (Status == EFI_TIMEOUT);
+  Status = ArmPlatformSysConfigSetDevice (
+             SYS_CFG_OSC_SITE1,
+             FixedPcdGet32 (PcdHdLcdVideoModeOscId),
+             mResolutions[ModeNumber].OscFreq
+             );
   if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     return Status;
   }
 
   // Set the DVI into the new mode
-  do {
-    Status = ArmPlatformSysConfigSet (
-               SYS_CFG_DVIMODE,
-               mResolutions[ModeNumber].Mode
-               );
-  } while (Status == EFI_TIMEOUT);
+  Status = ArmPlatformSysConfigSet (
+             SYS_CFG_DVIMODE,
+             mResolutions[ModeNumber].Mode
+             );
   if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     return Status;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 09/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (7 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 08/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check EFI_TIMEOUT Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 10/17] ARM/VExpressPkg: Redefine LcdPlatformGetTimings function Girish Pathak
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: EvanLloyd <evan.lloyd at arm.com>

Because of copy/paste effects, HdLcdArmVExpress.c contained a
table entry "LCD_BPP Bpp;" specifying the Bits per Pixel for each mode.
However, all modes are LCD_BITS_PER_PIXEL_24.

This change removes the table entry and related use of the field.

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c | 42 ++++++--------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index cd62cd61883dcd78c72aa614466c6f714cc73fc4..97cbe77ee79f53d3430b0fdb057a3bf262834849 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -32,7 +32,6 @@ typedef struct {
   UINT32                     Mode;
   UINT32                     HorizontalResolution;
   UINT32                     VerticalResolution;
-  LCD_BPP                    Bpp;
   UINT32                     OscFreq;
 
   // These are used by HDLCD
@@ -48,37 +47,37 @@ typedef struct {
 **/
 LCD_RESOLUTION mResolutions[] = {
   { // Mode 0 : VGA : 640 x 480 x 24 bpp
-    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS,
     VGA_OSC_FREQUENCY,
     VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
     VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
   },
   { // Mode 1 : SVGA : 800 x 600 x 24 bpp
-    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS,
     SVGA_OSC_FREQUENCY,
     SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
     SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
   },
   { // Mode 2 : XGA : 1024 x 768 x 24 bpp
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS,
     XGA_OSC_FREQUENCY,
     XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
     XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
   },
   { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
-    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS,
     (SXGA_OSC_FREQUENCY/2),
     SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
     SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
   },
   { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
-    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS,
     (UXGA_OSC_FREQUENCY/2),
     UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
     UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
   },
   { // Mode 5 : HD : 1920 x 1080 x 24 bpp
-    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS,
     (HD_OSC_FREQUENCY/2),
     HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
     HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
@@ -289,27 +288,12 @@ LcdPlatformQueryMode (
   Info->VerticalResolution = mResolutions[ModeNumber].VerticalResolution;
   Info->PixelsPerScanLine = mResolutions[ModeNumber].HorizontalResolution;
 
-  switch (mResolutions[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;
-  }
+  /* 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;
 
   return EFI_SUCCESS;
 }
@@ -392,7 +376,7 @@ LcdPlatformGetBpp (
     return EFI_INVALID_PARAMETER;
   }
 
-  *Bpp = mResolutions[ModeNumber].Bpp;
+  *Bpp = LCD_BITS_PER_PIXEL_24;
 
   return EFI_SUCCESS;
 }
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 10/17] ARM/VExpressPkg: Redefine LcdPlatformGetTimings function
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (8 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 09/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 11/17] ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format Girish Pathak
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: EvanLloyd <evan.lloyd at arm.com>

The LcdPlatformGetTimings interface function takes similar sets of
multiple parameters for horizontal and vertical timings which can be
aggregated in a common data type. This change defines a structure
SCAN_TIMINGS for this which can be used to describe both horizontal and
vertical scan timings, and accordingly redefines the
LcdPlatformGetTiming interface, greatly reducing the amount of data
passed about.

Similarly the mode definition tables are also changed to use this data
type and thus enable pass through access.

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c       | 106 +++++-------
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 168 ++++++++------------
 2 files changed, 109 insertions(+), 165 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index 97cbe77ee79f53d3430b0fdb057a3bf262834849..0c3a4efd6d8d1617965394f461c3f3e7bf70994d 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -30,57 +30,51 @@
 
 typedef struct {
   UINT32                     Mode;
-  UINT32                     HorizontalResolution;
-  UINT32                     VerticalResolution;
   UINT32                     OscFreq;
 
   // These are used by HDLCD
-  UINT32                     HSync;
-  UINT32                     HBackPorch;
-  UINT32                     HFrontPorch;
-  UINT32                     VSync;
-  UINT32                     VBackPorch;
-  UINT32                     VFrontPorch;
-} LCD_RESOLUTION;
+  SCAN_TIMINGS               Horizontal;
+  SCAN_TIMINGS               Vertical;
+} DISPLAY_MODE;
 
 /** The display modes supported by the platform.
 **/
-LCD_RESOLUTION mResolutions[] = {
+STATIC DISPLAY_MODE mDisplayModes[] = {
   { // Mode 0 : VGA : 640 x 480 x 24 bpp
-    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS,
+    VGA,
     VGA_OSC_FREQUENCY,
-    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
+    {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
   },
   { // Mode 1 : SVGA : 800 x 600 x 24 bpp
-    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS,
+    SVGA,
     SVGA_OSC_FREQUENCY,
-    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
+    {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
   },
   { // Mode 2 : XGA : 1024 x 768 x 24 bpp
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS,
+    XGA,
     XGA_OSC_FREQUENCY,
-    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
   },
   { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
-    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS,
+    SXGA,
     (SXGA_OSC_FREQUENCY/2),
-    SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
-    SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
+    {SXGA_H_RES_PIXELS, SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH},
+    {SXGA_V_RES_PIXELS, SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH}
   },
   { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
-    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS,
+    UXGA,
     (UXGA_OSC_FREQUENCY/2),
-    UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
-    UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
+    {UXGA_H_RES_PIXELS, UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH},
+    {UXGA_V_RES_PIXELS, UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH}
   },
   { // Mode 5 : HD : 1920 x 1080 x 24 bpp
-    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS,
+    HD,
     (HD_OSC_FREQUENCY/2),
-    HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
-    HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
+    {HD_H_RES_PIXELS, HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH},
+    {HD_V_RES_PIXELS, HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH}
   }
 };
 
@@ -205,7 +199,7 @@ LcdPlatformGetMaxMode (VOID)
 {
   // The following line will report correctly the total number of graphics modes
   // that could be supported by the graphics driver
-  return (sizeof (mResolutions) / sizeof (LCD_RESOLUTION));
+  return (sizeof (mDisplayModes) / sizeof (DISPLAY_MODE));
 }
 
 /** Set the requested display mode.
@@ -232,7 +226,7 @@ LcdPlatformSetMode (
   Status = ArmPlatformSysConfigSetDevice (
              SYS_CFG_OSC_SITE1,
              FixedPcdGet32 (PcdHdLcdVideoModeOscId),
-             mResolutions[ModeNumber].OscFreq
+             mDisplayModes[ModeNumber].OscFreq
              );
   if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
@@ -242,7 +236,7 @@ LcdPlatformSetMode (
   // Set the DVI into the new mode
   Status = ArmPlatformSysConfigSet (
              SYS_CFG_DVIMODE,
-             mResolutions[ModeNumber].Mode
+             mDisplayModes[ModeNumber].Mode
              );
   if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
@@ -284,9 +278,9 @@ LcdPlatformQueryMode (
   }
 
   Info->Version = 0;
-  Info->HorizontalResolution = mResolutions[ModeNumber].HorizontalResolution;
-  Info->VerticalResolution = mResolutions[ModeNumber].VerticalResolution;
-  Info->PixelsPerScanLine = mResolutions[ModeNumber].HorizontalResolution;
+  Info->HorizontalResolution = mDisplayModes[ModeNumber].Horizontal.Resolution;
+  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;
@@ -302,14 +296,10 @@ LcdPlatformQueryMode (
 
   @param[in]  ModeNumber          Mode Number.
 
-  @param[out] HRes                Pointer to horizontal resolution.
-  @param[out] HSync               Pointer to horizontal sync width.
-  @param[out] HBackPorch          Pointer to horizontal back porch.
-  @param[out] HFrontPorch         Pointer to horizontal front porch.
-  @param[out] VRes                Pointer to vertical resolution.
-  @param[out] VSync               Pointer to vertical sync width.
-  @param[out] VBackPorch          Pointer to vertical back porch.
-  @param[out] VFrontPorch         Pointer to vertical front porch.
+  @param[out] Horizontal          Pointer to horizontal timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
+  @param[out] Vertical            Pointer to vertical timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
 
   @retval EFI_SUCCESS             Display timing information for the requested
                                   mode returned successfully.
@@ -317,40 +307,22 @@ LcdPlatformQueryMode (
 **/
 EFI_STATUS
 LcdPlatformGetTimings (
-  IN  UINT32                              ModeNumber,
-  OUT UINT32*                             HRes,
-  OUT UINT32*                             HSync,
-  OUT UINT32*                             HBackPorch,
-  OUT UINT32*                             HFrontPorch,
-  OUT UINT32*                             VRes,
-  OUT UINT32*                             VSync,
-  OUT UINT32*                             VBackPorch,
-  OUT UINT32*                             VFrontPorch
+  IN  UINT32                  ModeNumber,
+  OUT SCAN_TIMINGS         ** Horizontal,
+  OUT SCAN_TIMINGS         ** Vertical
   )
 {
   // One of the pointers is NULL
-  ASSERT (HRes != NULL);
-  ASSERT (HSync != NULL);
-  ASSERT (HBackPorch != NULL);
-  ASSERT (HFrontPorch != NULL);
-  ASSERT (VRes != NULL);
-  ASSERT (VSync != NULL);
-  ASSERT (VBackPorch != NULL);
-  ASSERT (VFrontPorch != NULL);
+  ASSERT (Horizontal != NULL);
+  ASSERT (Vertical != NULL);
 
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
     ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
-  *HRes           = mResolutions[ModeNumber].HorizontalResolution;
-  *HSync          = mResolutions[ModeNumber].HSync;
-  *HBackPorch     = mResolutions[ModeNumber].HBackPorch;
-  *HFrontPorch    = mResolutions[ModeNumber].HFrontPorch;
-  *VRes           = mResolutions[ModeNumber].VerticalResolution;
-  *VSync          = mResolutions[ModeNumber].VSync;
-  *VBackPorch     = mResolutions[ModeNumber].VBackPorch;
-  *VFrontPorch    = mResolutions[ModeNumber].VFrontPorch;
+  *Horizontal = &mDisplayModes[ModeNumber].Horizontal;
+  *Vertical   = &mDisplayModes[ModeNumber].Vertical;
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index c51540e1c00a1f46ca845d95306e28e7ca6bfde5..a6fab279e3e5959228259e6d47703a304ad372a7 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -28,117 +28,111 @@
 
 typedef struct {
   UINT32                     Mode;
-  UINT32                     HorizontalResolution;
-  UINT32                     VerticalResolution;
   LCD_BPP                    Bpp;
   UINT32                     OscFreq;
 
-  UINT32                     HSync;
-  UINT32                     HBackPorch;
-  UINT32                     HFrontPorch;
-  UINT32                     VSync;
-  UINT32                     VBackPorch;
-  UINT32                     VFrontPorch;
-} LCD_RESOLUTION;
+  SCAN_TIMINGS               Horizontal;
+  SCAN_TIMINGS               Vertical;
+} DISPLAY_MODE;
 
 /** The display modes supported by the platform.
 **/
-LCD_RESOLUTION mResolutions[] = {
+STATIC DISPLAY_MODE mDisplayModes[] = {
   { // Mode 0 : VGA : 640 x 480 x 24 bpp
-    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    VGA, LCD_BITS_PER_PIXEL_24,
     VGA_OSC_FREQUENCY,
-    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
+    {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
   },
   { // Mode 1 : SVGA : 800 x 600 x 24 bpp
-    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    SVGA, LCD_BITS_PER_PIXEL_24,
     SVGA_OSC_FREQUENCY,
-    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
+    {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
   },
   { // Mode 2 : XGA : 1024 x 768 x 24 bpp
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    XGA, LCD_BITS_PER_PIXEL_24,
     XGA_OSC_FREQUENCY,
-    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
   },
   { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
-    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    SXGA, LCD_BITS_PER_PIXEL_24,
     (SXGA_OSC_FREQUENCY/2),
-    SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
-    SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
+    {SXGA_H_RES_PIXELS, SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH},
+    {SXGA_V_RES_PIXELS, SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH}
   },
   { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
-    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    UXGA, LCD_BITS_PER_PIXEL_24,
     (UXGA_OSC_FREQUENCY/2),
-    UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
-    UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
+    {UXGA_H_RES_PIXELS, UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH},
+    {UXGA_V_RES_PIXELS, UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH}
   },
   { // Mode 5 : HD : 1920 x 1080 x 24 bpp
-    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
+    HD, LCD_BITS_PER_PIXEL_24,
     (HD_OSC_FREQUENCY/2),
-    HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
-    HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
+    {HD_H_RES_PIXELS, HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH},
+    {HD_V_RES_PIXELS, HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH}
   },
   { // Mode 6 : VGA : 640 x 480 x 16 bpp (565 Mode)
-    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
+    VGA, LCD_BITS_PER_PIXEL_16_565,
     VGA_OSC_FREQUENCY,
-    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
+    {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
   },
   { // Mode 7 : SVGA : 800 x 600 x 16 bpp (565 Mode)
-    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
+    SVGA, LCD_BITS_PER_PIXEL_16_565,
     SVGA_OSC_FREQUENCY,
-    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
+    {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
   },
   { // Mode 8 : XGA : 1024 x 768 x 16 bpp (565 Mode)
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
+    XGA, LCD_BITS_PER_PIXEL_16_565,
     XGA_OSC_FREQUENCY,
-    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
   },
   { // Mode 9 : VGA : 640 x 480 x 15 bpp
-    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    VGA, LCD_BITS_PER_PIXEL_16_555,
     VGA_OSC_FREQUENCY,
-    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
+    {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
   },
   { // Mode 10 : SVGA : 800 x 600 x 15 bpp
-    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    SVGA, LCD_BITS_PER_PIXEL_16_555,
     SVGA_OSC_FREQUENCY,
-    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
+    {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
   },
   { // Mode 11 : XGA : 1024 x 768 x 15 bpp
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    XGA, LCD_BITS_PER_PIXEL_16_555,
     XGA_OSC_FREQUENCY,
-    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
   },
   { // Mode 12 : XGA : 1024 x 768 x 15 bpp - All the timing info is derived from Linux Kernel Driver Settings
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
+    XGA, LCD_BITS_PER_PIXEL_16_555,
     63500000,
-    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
   },
   { // Mode 13 : VGA : 640 x 480 x 12 bpp (444 Mode)
-    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
+    VGA, LCD_BITS_PER_PIXEL_12_444,
     VGA_OSC_FREQUENCY,
-    VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
-    VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
+    {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
+    {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
   },
   { // Mode 14 : SVGA : 800 x 600 x 12 bpp (444 Mode)
-    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
+    SVGA, LCD_BITS_PER_PIXEL_12_444,
     SVGA_OSC_FREQUENCY,
-    SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
-    SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
+    {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
+    {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
   },
   { // Mode 15 : XGA : 1024 x 768 x 12 bpp (444 Mode)
-    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
+    XGA, LCD_BITS_PER_PIXEL_12_444,
     XGA_OSC_FREQUENCY,
-    XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
-    XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
   }
 };
 
@@ -316,7 +310,7 @@ LcdPlatformSetMode (
   Status = ArmPlatformSysConfigSetDevice (
              Function,
              OscillatorId,
-             mResolutions[ModeNumber].OscFreq
+             mDisplayModes[ModeNumber].OscFreq
              );
   if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
@@ -334,7 +328,7 @@ LcdPlatformSetMode (
       // Set the DVI into the new mode
       Status = ArmPlatformSysConfigSet (
                  SYS_CFG_DVIMODE,
-                 mResolutions[ModeNumber].Mode
+                 mDisplayModes[ModeNumber].Mode
                  );
       if (EFI_ERROR (Status)) {
         ASSERT_EFI_ERROR (Status);
@@ -374,11 +368,11 @@ LcdPlatformQueryMode (
   }
 
   Info->Version = 0;
-  Info->HorizontalResolution = mResolutions[ModeNumber].HorizontalResolution;
-  Info->VerticalResolution = mResolutions[ModeNumber].VerticalResolution;
-  Info->PixelsPerScanLine = mResolutions[ModeNumber].HorizontalResolution;
+  Info->HorizontalResolution = mDisplayModes[ModeNumber].Horizontal.Resolution;
+  Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
+  Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
 
-  switch (mResolutions[ModeNumber].Bpp) {
+  switch (mDisplayModes[ModeNumber].Bpp) {
   case LCD_BITS_PER_PIXEL_24:
     Info->PixelFormat                   = PixelRedGreenBlueReserved8BitPerColor;
     Info->PixelInformation.RedMask      = LCD_24BPP_RED_MASK;
@@ -407,14 +401,10 @@ LcdPlatformQueryMode (
 
   @param[in]  ModeNumber          Mode Number.
 
-  @param[out] HRes                Pointer to horizontal resolution.
-  @param[out] HSync               Pointer to horizontal sync width.
-  @param[out] HBackPorch          Pointer to horizontal back porch.
-  @param[out] HFrontPorch         Pointer to horizontal front porch.
-  @param[out] VRes                Pointer to vertical resolution.
-  @param[out] VSync               Pointer to vertical sync width.
-  @param[out] VBackPorch          Pointer to vertical back porch.
-  @param[out] VFrontPorch         Pointer to vertical front porch.
+  @param[out] Horizontal          Pointer to horizontal timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
+  @param[out] Vertical            Pointer to vertical timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
 
   @retval EFI_SUCCESS             Display timing information for the requested
                                   mode returned successfully.
@@ -422,40 +412,22 @@ LcdPlatformQueryMode (
 **/
 EFI_STATUS
 LcdPlatformGetTimings (
-  IN  UINT32                              ModeNumber,
-  OUT UINT32*                             HRes,
-  OUT UINT32*                             HSync,
-  OUT UINT32*                             HBackPorch,
-  OUT UINT32*                             HFrontPorch,
-  OUT UINT32*                             VRes,
-  OUT UINT32*                             VSync,
-  OUT UINT32*                             VBackPorch,
-  OUT UINT32*                             VFrontPorch
+  IN  UINT32                  ModeNumber,
+  OUT SCAN_TIMINGS         ** Horizontal,
+  OUT SCAN_TIMINGS         ** Vertical
   )
 {
   // One of the pointers is NULL
-  ASSERT (HRes != NULL);
-  ASSERT (HSync != NULL);
-  ASSERT (HBackPorch != NULL);
-  ASSERT (HFrontPorch != NULL);
-  ASSERT (VRes != NULL);
-  ASSERT (VSync != NULL);
-  ASSERT (VBackPorch != NULL);
-  ASSERT (VFrontPorch != NULL);
+  ASSERT (Horizontal != NULL);
+  ASSERT (Vertical != NULL);
 
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
     ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
 
-  *HRes           = mResolutions[ModeNumber].HorizontalResolution;
-  *HSync          = mResolutions[ModeNumber].HSync;
-  *HBackPorch     = mResolutions[ModeNumber].HBackPorch;
-  *HFrontPorch    = mResolutions[ModeNumber].HFrontPorch;
-  *VRes           = mResolutions[ModeNumber].VerticalResolution;
-  *VSync          = mResolutions[ModeNumber].VSync;
-  *VBackPorch     = mResolutions[ModeNumber].VBackPorch;
-  *VFrontPorch    = mResolutions[ModeNumber].VFrontPorch;
+  *Horizontal = &mDisplayModes[ModeNumber].Horizontal;
+  *Vertical   = &mDisplayModes[ModeNumber].Vertical;
 
   return EFI_SUCCESS;
 }
@@ -483,7 +455,7 @@ LcdPlatformGetBpp (
     return EFI_INVALID_PARAMETER;
   }
 
-  *Bpp = mResolutions[ModeNumber].Bpp;
+  *Bpp = mDisplayModes[ModeNumber].Bpp;
 
   return EFI_SUCCESS;
 }
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 11/17] ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (9 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 10/17] ARM/VExpressPkg: Redefine LcdPlatformGetTimings function Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData Girish Pathak
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

Notes:
    v3:
    - Fix minor indentation                     [Ard]
    
      Done                                      [Girish

 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 23 ++++++++----
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  1 +
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 38 +++++++++-----------
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  1 +
 4 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index 0c3a4efd6d8d1617965394f461c3f3e7bf70994d..f7cae39c9cc9954ba4cad1bd597ebfc8baf10f11 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
@@ -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/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index eb2a3c94b80129a16bf9ae26b7c2a5403556dc71..9b0d358846bf367d7f9ff6f5d3fdffc204864528 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/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index a6fab279e3e5959228259e6d47703a304ad372a7..2f4814a2adbf01517ba14d75ce579ff35c362379 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
@@ -372,27 +388,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/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index f480000936f394accc98e8d031fcd2900a6f4611..2bf14f999e633a55abd572daaac1e80ae2e648eb 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
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (10 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 11/17] ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-21  3:37   ` Ard Biesheuvel
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 13/17] ARM/VExpressPkg: Reserving framebuffer at build Girish Pathak
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

As per the UEFI specification(2.7) section 12.9, the GOP framebuffer
memory can be accessed in the pre-boot and the post boot phase (by OS)
Therefore the memory type EfiBootServicesData is incorrect for
the framebuffer memory allocation. Change EfiBootServicesData with
EfiRuntimeServicesData flag so that allocated memory can be access
by the OS in the post boot phase.

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/HdLcdArmVExpress.c       | 2 +-
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index f7cae39c9cc9954ba4cad1bd597ebfc8baf10f11..c0a25a18d3fcfe91a76ee985ee58145b97900fa0 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -176,7 +176,7 @@ LcdPlatformGetVram (
   }
   Status = gBS->AllocatePages (
                   AllocationType,
-                  EfiBootServicesData,
+                  EfiRuntimeServicesData,
                   EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
                   VramBaseAddress
                   );
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 2f4814a2adbf01517ba14d75ce579ff35c362379..61ddf77e903e6c33a26b2aa8b76121e807195a9a 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -232,7 +232,7 @@ LcdPlatformGetVram (
     // Allocate the VRAM from the DRAM so that nobody else uses it.
     Status = gBS->AllocatePages (
                     AllocateAddress,
-                    EfiBootServicesData,
+                    EfiRuntimeServicesData,
                     EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
                     VramBaseAddress
                     );
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 13/17] ARM/VExpressPkg: Reserving framebuffer at build
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (11 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 14/17] ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer Girish Pathak
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

This change uses two PCDs, PcdArmLcdFrameBufferBase and
PcdArmLcdFrameBufferSize introduced in correspondiong EDK2 patch to
reserve framebuffer in DRAM if these values are defined in platform
specific DSC file, avoiding the need to allocate dynamically.
This allows the framebuffer to appear as "I/O memory" outside of the
normal RAM map, which is similar to the "VRAM" case.

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

Notes:
    v3:
    - Move PcdArmLcdDdrFrameBufferBase and
      PcdArmLcdDdrFrameBufferSize to VExpressPkg.                 [Ard]
    
      These PCDs are also used for the Juno platform hence these
      PCDs are defined for the ArmPlatformPkg so that both
      platform can use it.                                        [Girish]
    
    - Could you please add an ASSERT() so that System Memory
      and PcdArmLcdDdrFrameBufferBase do not overlap              [Ard]
    
      Done                                                        [Girish]

 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf |  4 +-
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c          | 41 ++++++++++++++------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
index 8c6291c42f8a599591d00d7afcb2ff3399417034..b025abd98b5e654323b7821ac353ad920e2e6421 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
@@ -1,5 +1,5 @@
 #/* @file
-#  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
+#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -54,6 +54,8 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdArmPrimaryCore
 
   gArmPlatformTokenSpaceGuid.PcdCoreCount
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
 
 [Ppis]
   gArmMpCoreInfoPpiGuid
diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
index 9fb0803d31ad0dbab59875bae99fd8a381d484b7..1d5fefc21726ba1b05d90e0e47677575d7fa2034 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+*  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
 *
 *  This program and the accompanying materials
 *  are licensed and made available under the terms and conditions of the BSD License
@@ -128,17 +128,34 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[Index].Length = 2 * ARM_VE_SMB_PERIPH_SZ;
   VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
-  // VRAM
-  VirtualMemoryTable[++Index].PhysicalBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE;
-  VirtualMemoryTable[Index].VirtualBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE;
-  VirtualMemoryTable[Index].Length = PL111_CLCD_VRAM_MOTHERBOARD_SIZE;
-  //
-  // Map the VRAM region as Normal Non-Cacheable memory and not device memory,
-  // so that we can use the accelerated string routines that may use unaligned
-  // accesses or DC ZVA instructions. The enum identifier is slightly awkward
-  // here, but it maps to a memory type that allows buffering and reordering.
-  //
-  VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
+  // Map region for the framebuffer in the system RAM if no VRAM present
+  if (FixedPcdGet32 (PcdArmLcdDdrFrameBufferBase) == 0) {
+    // VRAM
+    VirtualMemoryTable[++Index].PhysicalBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE;
+    VirtualMemoryTable[Index].VirtualBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE;
+    VirtualMemoryTable[Index].Length = PL111_CLCD_VRAM_MOTHERBOARD_SIZE;
+    //
+    // Map the VRAM region as Normal Non-Cacheable memory and not device memory,
+    // so that we can use the accelerated string routines that may use unaligned
+    // accesses or DC ZVA instructions. The enum identifier is slightly awkward
+    // here, but it maps to a memory type that allows buffering and reordering.
+    //
+    VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
+
+  } else {
+    ASSERT ((ARM_VE_DRAM_BASE + ARM_VE_DRAM_SZ - 1) <
+            FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase));
+    VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
+    VirtualMemoryTable[Index].VirtualBase = FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
+    VirtualMemoryTable[Index].Length = FixedPcdGet32 (PcdArmLcdDdrFrameBufferSize);
+    // Map as Normal Non-Cacheable memory, so that we can use the accelerated
+    // SetMem/CopyMem routines that may use unaligned accesses or
+    // DC ZVA instructions. If mapped as device memory, these routine may cause
+    // alignment faults.
+    // NOTE: The attribute value is misleading, it indicates memory map type as
+    // an un-cached, un-buffered but allows buffering and reordering.
+    VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
+  }
 
   // Map sparse memory region if present
   if (HasSparseMemory) {
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 14/17] ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (12 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 13/17] ARM/VExpressPkg: Reserving framebuffer at build Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 15/17] ARM/VExpressPkg: New DP500/DP550/DP650 platform library Girish Pathak
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

The framebuffer memory is set with flag
EFI_MEMORY_WC (uncached, unbuffered) which causes framebuffer memory
with eXecute bit set. Framebuffer memory having executable bit
set is a security hazard. This fix adds EFI_MEMORY_XP flag to avoid this.
Unfortunately function gDS->SetMemorySpaceAttributes() causes assertion due
to unsupported EFI_MEMORY_XP type. Therefore this fix replaces
gDS->SetMemorySpaceAttributes() with Cpu->SetMemoryAttributes().

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/HdLcdArmVExpress.c            | 24 ++++++++++++++------
 Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  1 -
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 24 ++++++++++++++------
 Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  1 -
 4 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index c0a25a18d3fcfe91a76ee985ee58145b97900fa0..4c114de4062ece7cee1221148afb42e66d04f07e 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -17,11 +17,11 @@
 #include <Library/ArmPlatformSysConfigLib.h>
 #include <Library/PcdLib.h>
 #include <Library/DebugLib.h>
-#include <Library/DxeServicesTableLib.h>
 #include <Library/LcdPlatformLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 
+#include <Protocol/Cpu.h>
 #include <Protocol/EdidDiscovered.h>
 #include <Protocol/EdidActive.h>
 
@@ -159,6 +159,7 @@ LcdPlatformGetVram (
 {
   EFI_STATUS              Status;
   EFI_ALLOCATE_TYPE       AllocationType;
+  EFI_CPU_ARCH_PROTOCOL   *Cpu;
 
   ASSERT (VramBaseAddress != NULL);
   ASSERT (VramSize != NULL);
@@ -185,13 +186,22 @@ LcdPlatformGetVram (
     return Status;
   }
 
-  // Mark the VRAM as write-combining.
-  // The VRAM is inside the DRAM, which is cacheable.
-  Status = gDS->SetMemorySpaceAttributes (
-                  *VramBaseAddress,
-                  *VramSize,
-                  EFI_MEMORY_WC
+  // Ensure the Cpu architectural protocol is already installed
+  Status = gBS->LocateProtocol (
+                  &gEfiCpuArchProtocolGuid,
+                  NULL,
+                  (VOID **)&Cpu
                   );
+  if (!EFI_ERROR (Status)) {
+    // The VRAM is inside the DRAM, which is cacheable.
+    // Mark the VRAM as write-combining (uncached) and non-executable.
+    Status = Cpu->SetMemoryAttributes (
+                    Cpu,
+                    *VramBaseAddress,
+                    *VramSize,
+                    EFI_MEMORY_WC | EFI_MEMORY_XP
+                    );
+  }
   if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
     gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index 9b0d358846bf367d7f9ff6f5d3fdffc204864528..c7b1b7fae77cbbf82b3a0768e7654a96719f5e7a 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -33,7 +33,6 @@ [Packages]
 [LibraryClasses]
   ArmPlatformSysConfigLib
   BaseLib
-  DxeServicesTableLib
 
 [Protocols]
   gEfiEdidDiscoveredProtocolGuid                # Produced
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 61ddf77e903e6c33a26b2aa8b76121e807195a9a..cae5f3a658efa4cc2be135259b63c860c26c6874 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -17,10 +17,10 @@
 #include <Library/IoLib.h>
 #include <Library/PcdLib.h>
 #include <Library/DebugLib.h>
-#include <Library/DxeServicesTableLib.h>
 #include <Library/LcdPlatformLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 
+#include <Protocol/Cpu.h>
 #include <Protocol/EdidDiscovered.h>
 #include <Protocol/EdidActive.h>
 
@@ -212,6 +212,7 @@ LcdPlatformGetVram (
   )
 {
   EFI_STATUS              Status;
+  EFI_CPU_ARCH_PROTOCOL   *Cpu;
 
   ASSERT (VramBaseAddress != NULL);
   ASSERT (VramSize != NULL);
@@ -241,13 +242,22 @@ LcdPlatformGetVram (
       return Status;
     }
 
-    // Mark the VRAM as write-combining.
-    // The VRAM is inside the DRAM, which is cacheable.
-    Status = gDS->SetMemorySpaceAttributes (
-                    *VramBaseAddress,
-                    *VramSize,
-                    EFI_MEMORY_WC
+    // Ensure the Cpu architectural protocol is already installed
+    Status = gBS->LocateProtocol (
+                    &gEfiCpuArchProtocolGuid,
+                    NULL,
+                    (VOID **)&Cpu
                     );
+    if (!EFI_ERROR (Status)) {
+      // The VRAM is inside the DRAM, which is cacheable.
+      // Mark the VRAM as write-combining (uncached) and non-executable.
+      Status = Cpu->SetMemoryAttributes (
+                      Cpu,
+                      *VramBaseAddress,
+                      *VramSize,
+                      EFI_MEMORY_WC | EFI_MEMORY_XP
+                      );
+    }
     if (EFI_ERROR (Status)) {
       ASSERT_EFI_ERROR (Status);
       gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index 2bf14f999e633a55abd572daaac1e80ae2e648eb..b1fa100def0dd774fec50cb04a638a89b95de575 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
@@ -33,7 +33,6 @@ [Packages]
 [LibraryClasses]
   ArmPlatformSysConfigLib
   BaseLib
-  DxeServicesTableLib
 
 [Protocols]
   gEfiEdidDiscoveredProtocolGuid                # Produced
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 15/17] ARM/VExpressPkg: New DP500/DP550/DP650 platform library
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (13 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 14/17] ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 16/17] ARM/JunoPkg: Adding SCMI MTL library Girish Pathak
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

From: Girish Pathak <girish.pathak at arm.com>

This change adds LcdPlatformLib implementation for Arm Mali
DP500/DP500/DP650 display processors for models (with DP550 support).

NOTE: Versions for actual hardware are liable to require extra handling
for clock input changes, etc.

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

Notes:
    v3:
    - Please fix the funky indentation       [Ard]
    
      Done                                   [Girish]
    
    - Add  EFI_MEMORY_XP                     [Ard]
    
      Done                                   [Girish]

 Platform/ARM/VExpressPkg/ArmVExpressPkg.dec                            |   3 +-
 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c           | 387 ++++++++++++++++++++
 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf         |  43 +++
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf |   3 +
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c          |  12 +-
 5 files changed, 446 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec b/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
index 695553a94f7f7e963b5db995c5e54f1ae1559daf..ac77540362d0775bed45f517306b98ea16bfb170 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
+++ b/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
@@ -1,7 +1,7 @@
 #/** @file
 # Arm Versatile Express package.
 #
-#  Copyright (c) 2012-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2012-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials are licensed and made available
 #  under the terms and conditions of the BSD License which accompanies this
@@ -51,6 +51,7 @@ [PcdsFixedAtBuild.common]
   gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId|1|UINT32|0x00000003
 
   gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId|0|UINT32|0x00000005
+  gArmVExpressTokenSpaceGuid.PcdArmMaliDpMaxMode|0|UINT32|0x00000008
 
   #
   # Device path of block device on which Fastboot will flash partitions
diff --git a/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..cc38e865d2246e12929ae65d1155a4e2ec6ac06f
--- /dev/null
+++ b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
@@ -0,0 +1,387 @@
+/** @file
+
+  The file contains Arm Mali DP platform specific implementation.
+
+  Copyright (c) 2017-2018, Arm Limited. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiDxe.h>
+#include <ArmPlatform.h>
+#include <Library/DebugLib.h>
+#include <Library/LcdPlatformLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/Cpu.h>
+
+/** Check an address is within 40 bits.
+
+  The ARM Mali DP framebuffer address size can not be wider than 40 bits
+**/
+#define  DP_VALID_BASE_ADDR(Address)  ((Address >> 40) == 0)
+
+typedef struct {
+  UINT32                      OscFreq;
+  SCAN_TIMINGS                Horizontal;
+  SCAN_TIMINGS                Vertical;
+} DISPLAY_MODE;
+
+/** The display modes implemented by this driver.
+
+  On Models, the OSC frequencies (listed for each mode below) are not used.
+  However these frequencies are useful on hardware plaforms where related
+  clock (or PLL) settings are based on these pixel clocks.
+
+  Since the clock settings are defined externally, the driver must
+  communicate pixel clock frequencies to relevant modules
+  responsible for setting clocks. e.g. SCP.
+**/
+STATIC DISPLAY_MODE mDisplayModes[] = {
+  {
+    // Mode 0 : VGA : 640 x 480 x 24 bpp.
+    VGA_OSC_FREQUENCY,
+    {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
+    {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
+  },
+  {
+    // Mode 1 : WVGA : 800 x 480 x 24 bpp.
+    WVGA_OSC_FREQUENCY,
+    {WVGA_H_RES_PIXELS, WVGA_H_SYNC, WVGA_H_BACK_PORCH, WVGA_H_FRONT_PORCH},
+    {WVGA_V_RES_PIXELS, WVGA_V_SYNC, WVGA_V_BACK_PORCH, WVGA_V_FRONT_PORCH}
+  },
+  {
+    // Mode 2 : SVGA : 800 x 600 x 24 bpp.
+    SVGA_OSC_FREQUENCY,
+    {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
+    {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
+  },
+  {
+    // Mode 3 : QHD : 960 x 540 x 24 bpp.
+    QHD_OSC_FREQUENCY,
+    {QHD_H_RES_PIXELS, QHD_H_SYNC, QHD_H_BACK_PORCH, QHD_H_FRONT_PORCH},
+    {QHD_V_RES_PIXELS, QHD_V_SYNC, QHD_V_BACK_PORCH, QHD_V_FRONT_PORCH}
+  },
+  {
+    // Mode 4 : WSVGA : 1024 x 600 x 24 bpp.
+    WSVGA_OSC_FREQUENCY,
+    {WSVGA_H_RES_PIXELS, WSVGA_H_SYNC, WSVGA_H_BACK_PORCH, WSVGA_H_FRONT_PORCH},
+    {WSVGA_V_RES_PIXELS, WSVGA_V_SYNC, WSVGA_V_BACK_PORCH, WSVGA_V_FRONT_PORCH}
+  },
+  {
+    // Mode 5 : XGA : 1024 x 768 x 24 bpp.
+    XGA_OSC_FREQUENCY,
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
+  },
+  {
+    // Mode 6 : HD : 1280 x 720 x 24 bpp.
+    HD720_OSC_FREQUENCY,
+    {HD720_H_RES_PIXELS, HD720_H_SYNC, HD720_H_BACK_PORCH, HD720_H_FRONT_PORCH},
+    {HD720_V_RES_PIXELS, HD720_V_SYNC, HD720_V_BACK_PORCH, HD720_V_FRONT_PORCH}
+  },
+  {
+    // Mode 7 : WXGA : 1280 x 800 x 24 bpp.
+    WXGA_OSC_FREQUENCY,
+    {WXGA_H_RES_PIXELS, WXGA_H_SYNC, WXGA_H_BACK_PORCH, WXGA_H_FRONT_PORCH},
+    {WXGA_V_RES_PIXELS, WXGA_V_SYNC, WXGA_V_BACK_PORCH, WXGA_V_FRONT_PORCH}
+  },
+  { // Mode 8 : SXGA : 1280 x 1024 x 24 bpp.
+    SXGA_OSC_FREQUENCY,
+    {SXGA_H_RES_PIXELS, SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH},
+    {SXGA_V_RES_PIXELS, SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH}
+  },
+  { // Mode 9 : WSXGA+ : 1680 x 1050 x 24 bpp.
+    WSXGA_OSC_FREQUENCY,
+    {WSXGA_H_RES_PIXELS, WSXGA_H_SYNC, WSXGA_H_BACK_PORCH, WSXGA_H_FRONT_PORCH},
+    {WSXGA_V_RES_PIXELS,WSXGA_V_SYNC, WSXGA_V_BACK_PORCH, WSXGA_V_FRONT_PORCH}
+  },
+  {
+    // Mode 10 : HD : 1920 x 1080 x 24 bpp.
+    HD_OSC_FREQUENCY,
+    {HD_H_RES_PIXELS, HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH},
+    {HD_V_RES_PIXELS, HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH}
+  }
+};
+
+/** If PcdArmMaliDpMaxMode is 0, platform supports full range of modes
+  else platform supports modes from 0 to PcdArmMaliDpMaxMode - 1
+**/
+STATIC CONST UINT32 mMaxMode = ((FixedPcdGet32 (PcdArmMaliDpMaxMode) != 0)
+                                   ? FixedPcdGet32 (PcdArmMaliDpMaxMode)
+                                   : sizeof (mDisplayModes) / sizeof (DISPLAY_MODE));
+
+/** Platform related initialization function.
+
+  @param[in] Handle             Handle to the instance of the device.
+
+  @retval EFI_SUCCESS           Initialization of platform library successful.
+  @retval EFI_UNSUPPORTED       PcdGopPixelFormat must be
+                                PixelRedGreenBlueReserved8BitPerColor OR
+                                PixelBlueGreenRedReserved8BitPerColor
+                                any other format is not supported.
+**/
+EFI_STATUS
+LcdPlatformInitializeDisplay (
+  IN EFI_HANDLE  Handle
+  )
+{
+  EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
+
+  (VOID)Handle;
+
+  // 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 EFI_SUCCESS;
+}
+
+/* Internal helper function to allocate memory if memory is not already
+  reserved for framebuffer
+
+  @param[in]  VramSize          Requested framebuffer size in bytes.
+  @param[out] VramBaseAddress   Pointer to memory allocated for framebuffer.
+
+  @retval EFI_SUCCESS           Framebuffer memory allocated successfully.
+  @retval EFI_UNSUPPORTED       Allocated address wider than 40 bits.
+  @retval !EFI_SUCCESS          Other errors.
+**/
+STATIC
+EFI_STATUS
+GetVram (
+  IN  UINTN                   VramSize,
+  OUT EFI_PHYSICAL_ADDRESS *  VramBaseAddress
+  )
+{
+  EFI_STATUS             Status;
+  EFI_CPU_ARCH_PROTOCOL  *Cpu;
+
+  *VramBaseAddress = FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
+
+  // Check if memory is already reserved for the framebuffer.
+  if (*VramBaseAddress != 0) {
+    // ARM Mali DP framebuffer base address can not be wider than 40 bits.
+    if (!DP_VALID_BASE_ADDR (*VramBaseAddress)) {
+      ASSERT (DP_VALID_BASE_ADDR (*VramBaseAddress));
+      Status = EFI_UNSUPPORTED;
+    } else {
+      Status = EFI_SUCCESS;
+    }
+  } else {
+    // If not already reserved, attempt to allocate the VRAM from the DRAM.
+    Status = gBS->AllocatePages (
+                    AllocateAnyPages,
+                    EfiRuntimeServicesData,
+                    EFI_SIZE_TO_PAGES (VramSize),
+                    VramBaseAddress
+                    );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate framebuffer.\n"));
+      ASSERT_EFI_ERROR (Status);
+      return Status;
+    }
+
+    // ARM Mali DP framebuffer base address can not be wider than 40 bits.
+    if (!DP_VALID_BASE_ADDR (*VramBaseAddress)) {
+      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (VramSize));
+      ASSERT (DP_VALID_BASE_ADDR (*VramBaseAddress));
+      return EFI_UNSUPPORTED;
+    }
+
+    // Ensure the Cpu architectural protocol is already installed
+    Status = gBS->LocateProtocol (
+                    &gEfiCpuArchProtocolGuid,
+                    NULL,
+                    (VOID **)&Cpu
+                    );
+    if (!EFI_ERROR (Status)) {
+      // The VRAM is inside the DRAM, which is cacheable.
+      // Mark the VRAM as write-combining (uncached) and non-executable.
+      Status = Cpu->SetMemoryAttributes (
+                      Cpu,
+                      *VramBaseAddress,
+                      VramSize,
+                      EFI_MEMORY_WC | EFI_MEMORY_XP
+                      );
+    }
+    if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
+      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (VramSize));
+    }
+  }
+
+  return Status;
+}
+
+/** Allocate VRAM memory in DRAM for the framebuffer
+  (unless it is reserved already).
+
+  The allocated address can be used to set the framebuffer as a base buffer
+  address for any layer of the ARM Mali DP.
+
+  @param[out] VramBaseAddress     A pointer to the framebuffer address.
+  @param[out] VramSize            A pointer to the size of the frame
+                                  buffer in bytes
+
+  @retval EFI_SUCCESS             Framebuffer memory allocation success.
+  @retval EFI_UNSUPPORTED         Allocated address wider than 40 bits
+  @retval !EFI_SUCCESS            Other errors.
+**/
+EFI_STATUS
+LcdPlatformGetVram (
+  OUT EFI_PHYSICAL_ADDRESS * VramBaseAddress,
+  OUT UINTN                * VramSize
+  )
+{
+  ASSERT (VramBaseAddress != NULL);
+  ASSERT (VramSize != NULL);
+
+  // Set the VRAM size.
+  *VramSize = (UINTN)FixedPcdGet32 (PcdArmLcdDdrFrameBufferSize);
+
+  return GetVram (*VramSize, VramBaseAddress);
+}
+
+/** Return total number of modes supported.
+
+  Note: Valid mode numbers are 0 to MaxMode - 1
+  See Section 12.9 of the UEFI Specification 2.7
+
+  @retval UINT32             Mode Number.
+**/
+UINT32
+LcdPlatformGetMaxMode (VOID)
+{
+  return  mMaxMode;
+}
+
+/** Set the requested display mode.
+
+  @param[in] ModeNumber            Mode Number.
+
+  @retval EFI_SUCCESS              Mode set successful.
+  @retval EFI_INVALID_PARAMETER    Requested mode not found.
+**/
+EFI_STATUS
+LcdPlatformSetMode (
+  IN UINT32 ModeNumber
+  )
+{
+  if (ModeNumber >= mMaxMode) {
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // On models, platform specific clock/mux settings are not required.
+  // Display controller specific settings for Mali DP are done in LcdSetMode.
+  return EFI_SUCCESS;
+}
+
+/** Return information for the requested mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+  @param[out] Info                Pointer for returned mode information
+                                  (on success).
+
+  @retval EFI_SUCCESS             Display mode information of the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
+EFI_STATUS
+LcdPlatformQueryMode (
+  IN  UINT32                                 ModeNumber,
+  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * Info
+  )
+{
+  if (ModeNumber >= mMaxMode) {
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ASSERT (Info != NULL);
+
+  Info->Version = 0;
+  Info->HorizontalResolution = mDisplayModes[ModeNumber].Horizontal.Resolution;
+  Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
+  Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
+
+  Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
+
+  return EFI_SUCCESS;
+}
+
+/** Return the display timing information for the requested mode number.
+
+  @param[in]  ModeNumber         Mode Number.
+
+  @param[out] Horizontal         Pointer to horizontal timing parameters.
+                                 (Resolution, Sync, Back porch, Front porch)
+  @param[out] Vertical           Pointer to vertical timing parameters.
+                                 (Resolution, Sync, Back porch, Front porch)
+
+  @retval EFI_SUCCESS             Display timing information of the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
+EFI_STATUS
+LcdPlatformGetTimings (
+  IN  UINT32                ModeNumber,
+  OUT SCAN_TIMINGS          ** Horizontal,
+  OUT SCAN_TIMINGS          ** Vertical
+  )
+{
+  ASSERT (Horizontal != NULL);
+  ASSERT (Vertical != NULL);
+
+  if (ModeNumber >= mMaxMode) {
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Horizontal = &mDisplayModes[ModeNumber].Horizontal;
+  *Vertical = &mDisplayModes[ModeNumber].Vertical;
+
+  return EFI_SUCCESS;
+}
+
+/** Return bits per pixel information for a mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+  @param[out] Bpp                 Pointer to bits per pixel information.
+
+  @retval EFI_SUCCESS             Bits per pixel information of the display
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
+EFI_STATUS
+LcdPlatformGetBpp (
+  IN  UINT32    ModeNumber,
+  OUT LCD_BPP * Bpp
+  )
+{
+  ASSERT (Bpp != NULL);
+  // Check valid ModeNumber.
+  if (ModeNumber >= mMaxMode) {
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Bpp = LCD_BITS_PER_PIXEL_24;
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..78f7d307b70701194b7121f16529ab10a6d52835
--- /dev/null
+++ b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
@@ -0,0 +1,43 @@
+#/** @file
+#  Component description file for ArmMaliDpLib module
+#
+#  Copyright (c) 2017-2018, Arm Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010019
+  BASE_NAME                      = ArmMaliDpLib
+  FILE_GUID                      = 36C47FED-2F3F-49C7-89CE-31B13F0431D8
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = LcdPlatformLib
+
+[Sources.common]
+  ArmMaliDpLib.c
+
+[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
+
+[LibraryClasses]
+  BaseLib
+
+[FixedPcd.common]
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
+  gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
+
+  # MaxMode must be one number higher than the actual max mode,
+  # i.e. for actual maximum mode 2, set the value to 3.
+  # See Section 12.9 of the UEFI Specification 2.7
+  gArmVExpressTokenSpaceGuid.PcdArmMaliDpMaxMode
+
diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
index b025abd98b5e654323b7821ac353ad920e2e6421..53898c5e957eb1a49cf063fa759d5a9ca62e7954 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
@@ -57,5 +57,8 @@ [FixedPcd]
   gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
   gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
 
+  gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase
+  gArmPlatformTokenSpaceGuid.PcdArmMaliDpMemoryRegionLength
+
 [Ppis]
   gArmMpCoreInfoPpiGuid
diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
index 1d5fefc21726ba1b05d90e0e47677575d7fa2034..c8eefa0cf28b06d19929912d28d87bd78ed7d871 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
@@ -20,8 +20,10 @@
 #include <Library/MemoryAllocationLib.h>
 #include <ArmPlatform.h>
 
+#define DP_BASE_DESCRIPTOR      ((FixedPcdGet64 (PcdArmMaliDpBase) != 0) ? 1 : 0)
+
 // Number of Virtual Memory Map Descriptors
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          9
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + DP_BASE_DESCRIPTOR)
 
 // DDR attributes
 #define DDR_ATTRIBUTES_CACHED   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
@@ -157,6 +159,14 @@ ArmPlatformGetVirtualMemoryMap (
     VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
   }
 
+  if (FixedPcdGet64 (PcdArmMaliDpBase) != 0) {
+    // DP500/DP550/DP650 peripheral memory region
+    VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 (PcdArmMaliDpBase);
+    VirtualMemoryTable[Index].VirtualBase = FixedPcdGet64 (PcdArmMaliDpBase);
+    VirtualMemoryTable[Index].Length = FixedPcdGet32 (PcdArmMaliDpMemoryRegionLength);
+    VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  }
+
   // Map sparse memory region if present
   if (HasSparseMemory) {
     VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 16/17] ARM/JunoPkg: Adding SCMI MTL library
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (14 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 15/17] ARM/VExpressPkg: New DP500/DP550/DP650 platform library Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 17/17] ARM/JunoPkg: Add HDLCD platform library Girish Pathak
  2018-03-21 12:56 ` [PATCH edk2-platforms v3 00/17] Update GOP Evan Lloyd
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

This change adds a new Mailbox Transport Layer library for the Juno
platform. This library is required for ArmScmiDxe driver communication
with the SCP.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Girish Pathak <girish.pathak@arm.com>
---

Notes:
    v3:
    - Please rename this library *instance* to something
      Juno-specific, e.g., ArmJunoMtlLib                    [Ard]
    
      Renamed to ArmJunoMtlLib                              [Girish]

 Platform/ARM/JunoPkg/ArmJuno.dec                                  |   9 +-
 Platform/ARM/JunoPkg/ArmJuno.dsc                                  |   5 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c              |   8 +-
 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c        | 198 ++++++++++++++++++++
 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf      |  39 ++++
 Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h |  94 ++++++++++
 6 files changed, 349 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec b/Platform/ARM/JunoPkg/ArmJuno.dec
index 60cef6d23a2d904103b9806d871fd2b89fff51c3..fdb56fccb2b259301fd787016a9c4de7ab3c356d 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dec
+++ b/Platform/ARM/JunoPkg/ArmJuno.dec
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2013-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -46,3 +46,10 @@ [PcdsFixedAtBuild.common]
   # Juno Device Trees are loaded from NOR Flash
   gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/board.dtb"|VOID*|0x00000008
 
+  # MHU Register base used by SCMI Mailbox transport
+  gArmJunoTokenSpaceGuid.PcdArmMtlDoorBell|0x2B1F0000|UINT64|0x00000024
+
+  # ARM_JUNO_NON_SECURE_SRAM_BASE used by SCMI Mailbox transport
+  gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxBase|0x2E000000|UINT64|0x00000025
+  gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxSize|0x80|UINT32|0x00000026
+
diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc b/Platform/ARM/JunoPkg/ArmJuno.dsc
index 9d7317683ef39ab47429234b98d94c04953b41cb..851dc0f04ad693e4a88cee070b205f0234687d81 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dsc
+++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
+#  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -47,6 +47,9 @@ [LibraryClasses.common]
   # USB Requirements
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
 
+  # SCMI Mailbox Transport Layer
+  ArmMtlLib|Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
+
 [LibraryClasses.common.SEC]
   PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
   ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
index 2d9c2c95a80d9c34ba4a1a526950537a97ec5d10..2cac815e96104e74c9ceae2c4140bcab535432a8 100644
--- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2013-2015, ARM Limited. All rights reserved.
+*  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 *
 *  This program and the accompanying materials
 *  are licensed and made available under the terms and conditions of the BSD License
@@ -107,7 +107,11 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[++Index].PhysicalBase  = ARM_JUNO_NON_SECURE_SRAM_BASE;
   VirtualMemoryTable[Index].VirtualBase     = ARM_JUNO_NON_SECURE_SRAM_BASE;
   VirtualMemoryTable[Index].Length          = ARM_JUNO_NON_SECURE_SRAM_SZ;
-  VirtualMemoryTable[Index].Attributes      = CacheAttributes;
+  // This memory is shared between the application processor
+  // and the SCP. To avoid coherency problems, map it as uncached memory.
+  // NOTE: The attribute value is misleading, it indicates memory map type as
+  // an un-cached, un-buffered but allows buffering and reordering.
+  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
 
   // PCI Root Complex
   VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdPcieControlBaseAddress);
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c b/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..ee1efe1574c124c7f7cf1fb345b46806bc3a4466
--- /dev/null
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c
@@ -0,0 +1,198 @@
+/** @file
+
+  Copyright (c) 2017-2018, Arm Limited. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  System Control and Management Interface V1.0
+    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
+    DEN0056A_System_Control_and_Management_Interface.pdf
+**/
+
+#include <Uefi/UefiBaseType.h>
+#include <Uefi/UefiSpec.h>
+#include <Library/ArmLib.h>
+#include <Library/ArmMtlLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+#include "ArmJunoMtlPrivateLib.h"
+
+// Each channel has a shared mailbox and a doorbell register.
+STATIC CONST MTL_CHANNEL Channels[NUM_CHANNELS] = {
+    // Low priority channel.
+    {
+      MTL_CHANNEL_TYPE_LOW,
+      (MTL_MAILBOX*)(MTL_MAILBOX_BASE),
+      DOORBELL_LOW
+    },
+    // High priority channel
+    {
+      MTL_CHANNEL_TYPE_HIGH,
+      (MTL_MAILBOX*)(MTL_MAILBOX_BASE + MTL_MAILBOX_HIGH_PRIORITY_OFFSET),
+      DOORBELL_HIGH
+    }
+  };
+
+/** Wait until channel is free.
+
+  @param[in] Channel                Pointer to a channel.
+  @param[in] TimeOutInMicroSeconds  Time out in micro seconds.
+
+  @retval EFI_SUCCESS               Channel is free.
+  @retval EFI_TIMEOUT               Time out error.
+**/
+EFI_STATUS
+MtlWaitUntilChannelFree (
+  IN MTL_CHANNEL  *Channel,
+  IN UINTN        TimeOutInMicroSeconds
+  )
+{
+  while (TimeOutInMicroSeconds != 0) {
+    // If channel is free then we have received the reply.
+    if (Channel->MailBox->ChannelStatus == MTL_CHANNEL_FREE) {
+      return EFI_SUCCESS;
+    }
+    if (TimeOutInMicroSeconds < MTL_POLL_WAIT_TIME) {
+      gBS->Stall (TimeOutInMicroSeconds);
+      break;
+    }
+    // Wait for some arbitrary time.
+    gBS->Stall (MTL_POLL_WAIT_TIME);
+    TimeOutInMicroSeconds -= MTL_POLL_WAIT_TIME;
+  }
+
+  // No response from SCP.
+  if (Channel->MailBox->ChannelStatus != MTL_CHANNEL_FREE) {
+    ASSERT (FALSE);
+    return EFI_TIMEOUT;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/** Return the address of the message payload.
+
+  @param[in] Channel   Pointer to a channel.
+
+  @retval UINT32*      Pointer to the payload.
+**/
+UINT32*
+MtlGetChannelPayload (
+  IN  MTL_CHANNEL  *Channel
+  )
+{
+  return Channel->MailBox->Payload;
+}
+
+/** Return pointer to a channel for the requested channel type.
+
+  @param[in] ChannelType        ChannelType, Low or High priority channel.
+                                MTL_CHANNEL_TYPE_LOW or
+                                MTL_CHANNEL_TYPE_HIGH
+
+  @param[out] Channel           Holds pointer to the channel.
+
+  @retval EFI_SUCCESS           Pointer to channel is returned.
+  @retval EFI_UNSUPPORTED       Requested channel type not supported.
+**/
+EFI_STATUS
+MtlGetChannel (
+  IN  MTL_CHANNEL_TYPE  ChannelType,
+  OUT MTL_CHANNEL       **Channel
+  )
+{
+  if (ChannelType != MTL_CHANNEL_TYPE_LOW
+    && ChannelType != MTL_CHANNEL_TYPE_HIGH) {
+    return EFI_UNSUPPORTED;
+  }
+
+  *Channel = (MTL_CHANNEL*)&Channels[ChannelType];
+
+  return EFI_SUCCESS;
+}
+
+/** Mark the channel busy and ring the doorbell.
+
+  @param[in] Channel               Pointer to a channel.
+  @param[in] MessageHeader         Message header.
+
+  @param[out] PayloadLength        Message length.
+
+  @retval EFI_SUCCESS              Message sent successfully.
+  @retval EFI_DEVICE_ERROR         Channel is busy.
+**/
+EFI_STATUS
+MtlSendMessage (
+  IN  MTL_CHANNEL  *Channel,
+  IN  UINT32       MessageHeader,
+  OUT UINT32       PayloadLength
+  )
+{
+  MTL_MAILBOX *MailBox = Channel->MailBox;
+
+  if (Channel->MailBox->ChannelStatus != MTL_CHANNEL_FREE) {
+    return EFI_DEVICE_ERROR;
+  }
+
+  // Mark the channel busy before ringing doorbell.
+  Channel->MailBox->ChannelStatus = MTL_CHANNEL_BUSY;
+  ArmDataSynchronizationBarrier ();
+
+  MailBox->Flags         = MTL_POLL;
+  MailBox->MessageHeader = MessageHeader;
+
+  // Add length of message header.
+  MailBox->Length = PayloadLength + sizeof (MessageHeader);
+
+  // Ring the doorbell. It sets SET bit of the MHU register.
+  MmioWrite32 (
+    Channel->DoorBell.PhysicalAddress,
+    Channel->DoorBell.ModifyMask
+    );
+
+  return EFI_SUCCESS;
+}
+
+/** Wait for a response on a channel.
+
+  If channel is free after sending message, it implies SCP responded
+  with a response on the channel.
+
+  @param[in] Channel               Pointer to a channel.
+
+  @retval EFI_SUCCESS              Message received successfully.
+  @retval EFI_TIMEOUT              Time out error.
+**/
+EFI_STATUS
+MtlReceiveMessage (
+  IN  MTL_CHANNEL  *Channel,
+  OUT UINT32       *MessageHeader,
+  OUT UINT32       *PayloadLength
+  )
+{
+  EFI_STATUS Status;
+
+  MTL_MAILBOX *MailBox = Channel->MailBox;
+
+  Status = MtlWaitUntilChannelFree (Channel, RESPONSE_TIMEOUT);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  *MessageHeader = MailBox->MessageHeader;
+
+  // Deduct message header length.
+  *PayloadLength = MailBox->Length - sizeof (*MessageHeader);
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf b/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..4b46c8071db1dbb5c740576c75461f65216c7a95
--- /dev/null
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
@@ -0,0 +1,39 @@
+#/** @file
+#  Copyright (c) 2017-2018, Arm Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010019
+  BASE_NAME                      = ArmJunoMtlLib
+  FILE_GUID                      = 21FB2D8F-C6C8-4B2C-A616-A30CB2FBA277
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ArmMtlLib
+
+[Sources.common]
+  ArmJunoMtlLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/JunoPkg/ArmJuno.dec
+
+[LibraryClasses]
+  ArmLib
+  DebugLib
+  IoLib
+  UefiBootServicesTableLib
+
+[FixedPcd.common]
+  gArmJunoTokenSpaceGuid.PcdArmMtlDoorBell
+  gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxBase
+  gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxSize
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h b/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h
new file mode 100644
index 0000000000000000000000000000000000000000..b2bac5e513e93b412307bff24298bbecc083d30c
--- /dev/null
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h
@@ -0,0 +1,94 @@
+/** @file
+
+  Copyright (c) 2017-2018, Arm Limited. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  System Control and Management Interface V1.0
+    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
+    DEN0056A_System_Control_and_Management_Interface.pdf
+
+  Juno ARM Development Platform SoC
+    https://www.arm.com/files/pdf/
+    DDI0515D1a_juno_arm_development_platform_soc_trm.pdf
+**/
+
+#ifndef ARM_JUNO_MTL_PRIVATE_LIB_H_
+#define ARM_JUNO_MTL_PRIVATE_LIB_H_
+
+// Mailbox transport layer.
+#define MTL_DOORBELL_MODIFY_MASK   (0x00000001U)
+#define MTL_DOORBELL_PRESERVE_MASK (~MTL_DOORBELL_MODIFY_MASK)
+
+#define MTL_DOORBELL_BASE    (FixedPcdGet64 (PcdArmMtlDoorBell))
+#define MTL_MAILBOX_BASE     (FixedPcdGet64 (PcdArmMtlMailBoxBase))
+#define MTL_MAILBOX_SIZE     (FixedPcdGet32 (PcdArmMtlMailBoxSize))
+
+#define MTL_POLL         0
+#define MTL_INTR         1
+
+/* For Juno, the mailbox for high priority is non-trusted SRAM + 256.
+
+   NOTE: Below is not documented anywhere (yet)
+
+   The payload sizes are 128 bytes.
+
+   There are two channels:
+
+   Channel 0
+    - Agent (OS) to Platform (SCP) memory base: non-trusted SRAM + 0
+    - Platform (SCP) to Agent (OS) memory base: non-trusted SRAM + 128
+    - Doorbell (both directions): MHU, bit 0
+
+   Channel 1
+    - Agent (OS) to Platform (SCP) memory base: non-trusted SRAM + 256
+    - Platform (SCP) to Agent (OS) memory base: non-trusted SRAM + 384
+    - Doorbell (both directions): MHU, bit 0
+*/
+#define MTL_MAILBOX_HIGH_PRIORITY_OFFSET (MTL_MAILBOX_SIZE * 2)
+
+// ARM MHU interrupt registers.
+#define CPU_INTR_L_SET  0x108
+#define CPU_INTR_H_SET  0x128
+
+// MTL uses MHU interrupt registers for communication with the SCP.
+#define MTL_DOORBELL_REGISTER_LOW   (MTL_DOORBELL_BASE + CPU_INTR_L_SET)
+#define MTL_DOORBELL_REGISTER_HIGH  (MTL_DOORBELL_BASE + CPU_INTR_H_SET)
+
+#define MTL_CHANNEL_BUSY    0
+#define MTL_CHANNEL_FREE    1
+
+// Response time out value on a MHU channel 20ms.
+#define  RESPONSE_TIMEOUT  20000
+
+/* As per SCMI spec. as a agent UEFI(or OS) can access only two channels
+   (low or high priority) secure channel is only accessible
+   to ARM Trusted firmware. */
+#define  NUM_CHANNELS      2
+
+/* Each channel must use a doorbell register to interrupt the SCP firmware.
+   on Juno these are MHU interrupt registers for low and high priority
+   channels. */
+#define  DOORBELL_LOW   {                                \
+                          MTL_DOORBELL_REGISTER_LOW,     \
+                          MTL_DOORBELL_MODIFY_MASK,      \
+                          MTL_DOORBELL_PRESERVE_MASK     \
+                        }
+
+#define  DOORBELL_HIGH  {                                \
+                          MTL_DOORBELL_REGISTER_HIGH,    \
+                          MTL_DOORBELL_MODIFY_MASK,      \
+                          MTL_DOORBELL_PRESERVE_MASK     \
+                        }
+
+// Arbitarary poll time.
+#define MTL_POLL_WAIT_TIME 100
+
+#endif /* ARM_JUNO_MTL_PRIVATE_LIB_H_ */
+
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* [PATCH edk2-platforms v3 17/17] ARM/JunoPkg: Add HDLCD platform library
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (15 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 16/17] ARM/JunoPkg: Adding SCMI MTL library Girish Pathak
@ 2018-03-20 16:18 ` Girish Pathak
  2018-03-21 12:56 ` [PATCH edk2-platforms v3 00/17] Update GOP Evan Lloyd
  17 siblings, 0 replies; 24+ messages in thread
From: Girish Pathak @ 2018-03-20 16:18 UTC (permalink / raw)
  To: edk2-devel
  Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind.Chauhan, Daniil.Egranov,
	thomas.abraham

This change adds the HDLCD platform lib for the Juno plaform. This
library will be instantiated as a LcdPlatformLib to link with
LcdGraphicsOutputDxe for the Juno platform.

HDLCD platform library depends on the Arm SCMI DXE driver for
communication with the SCP for clock setting. Therefore this change also
enables building of Arm SCMI DXE driver for the Juno 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>
---

Notes:
    v3:
    - Wouldn't it make more sense to add a macro
      ENABLE_HDLCD, rather than inverting
      the logic?                                    [Ard]
    
      We used this to correspond with the ACPI
      (FADT) terminology, where HEADLESS implies
      more than just no display.
      Refer below email                             [Evan]
      https://lists.01.org/pipermail/edk2-devel/
      2018-January/019925.html
    
    - SRAM mapping: Couldn't you map it as
      non-cacheable memory instead                  [Ard]
    
      Done                                          [Girish]
    
    - Please fix weird indentation                  [Ard]
    
      Done                                          [Girish]
    
    - Please don't use CPP conditionals for
      control flow                                  [Ard]
    
      Done                                          [Girish]

 Platform/ARM/JunoPkg/ArmJuno.dec                                 |   8 +
 Platform/ARM/JunoPkg/ArmJuno.dsc                                 |  26 +
 Platform/ARM/JunoPkg/ArmJuno.fdf                                 |  12 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf           |   5 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c             |  21 +-
 Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c      | 555 ++++++++++++++++++++
 Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf |  41 ++
 7 files changed, 665 insertions(+), 3 deletions(-)

diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec b/Platform/ARM/JunoPkg/ArmJuno.dec
index fdb56fccb2b259301fd787016a9c4de7ab3c356d..edbbb827ad45cf9e33adede006628432aa41c77b 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dec
+++ b/Platform/ARM/JunoPkg/ArmJuno.dec
@@ -53,3 +53,11 @@ [PcdsFixedAtBuild.common]
   gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxBase|0x2E000000|UINT64|0x00000025
   gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxSize|0x80|UINT32|0x00000026
 
+  # MaxMode must be one number higher than the actual max mode,
+  # i.e. for actual maximum mode 2, set the value to 3.
+  #
+  # Default value zero allows platform to enumerate maximum supported mode.
+  #
+  # For a list of mode numbers look in HdLcdArmJuno.c
+  gArmJunoTokenSpaceGuid.PcdArmHdLcdMaxMode|0|UINT32|0x00000017
+
diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc b/Platform/ARM/JunoPkg/ArmJuno.dsc
index 851dc0f04ad693e4a88cee070b205f0234687d81..d5e87f1edfd5d60a543e51cb42dfbcc4489d3f7d 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dsc
+++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
@@ -50,6 +50,11 @@ [LibraryClasses.common]
   # SCMI Mailbox Transport Layer
   ArmMtlLib|Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
 
+!ifndef HEADLESS_PLATFORM
+  LcdPlatformLib|Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
+  LcdHwLib|ArmPlatformPkg/Library/HdLcd/HdLcd.inf
+!endif
+
 [LibraryClasses.common.SEC]
   PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
   ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
@@ -100,7 +105,15 @@ [PcdsFixedAtBuild.common]
 
   # System Memory (2GB - 16MB of Trusted DRAM at the top of the 32bit address space)
   gArmTokenSpaceGuid.PcdSystemMemoryBase|0x80000000
+
+!ifdef HEADLESS_PLATFORM
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x7F000000
+!else
+  # Default framebuffer size is 0x7E9000, reduce system memory size for framebuffer.
+  gArmTokenSpaceGuid.PcdSystemMemorySize|0x7E817000
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase|0xFE817000
+  gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect|TRUE
+!endif
 
   # Juno Dual-Cluster profile
   gArmPlatformTokenSpaceGuid.PcdCoreCount|6
@@ -142,6 +155,11 @@ [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdGicDistributorBase|0x2C010000
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x2C02F000
 
+!ifndef HEADLESS_PLATFORM
+  # ARM Juno HDLCD Base
+  gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase|0x7FF60000
+!endif
+
   #
   # PLDA PCI Root Complex
   #
@@ -315,6 +333,11 @@ [Components.common]
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
   MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
 
+!ifndef HEADLESS_PLATFORM
+  # Graphic Output Protocol
+  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf
+!endif
+
   #
   # Juno platform driver
   #
@@ -348,6 +371,9 @@ [Components.common]
       BdsLib|Platform/ARM/Library/BdsLib/BdsLib.inf
   }
 
+  # SCMI Driver
+  ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
+
 [Components.AARCH64]
   #
   # EBC
diff --git a/Platform/ARM/JunoPkg/ArmJuno.fdf b/Platform/ARM/JunoPkg/ArmJuno.fdf
index ee9d0e7f4f6e6ac99ded6a14e88eb2c7854dd473..c538a9b754995bbc1c4657ecef2eefdcdd907da5 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.fdf
+++ b/Platform/ARM/JunoPkg/ArmJuno.fdf
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2013-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -163,6 +163,13 @@ [FV.FvMain]
   INF MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouseDxe.inf
   INF MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
 
+!ifndef HEADLESS_PLATFORM
+  #
+  # Graphics Output Protocol
+  #
+  INF ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf
+!endif
+
   #
   # PCI Support
   #
@@ -223,6 +230,9 @@ [FV.FvMain]
   # after the device drivers (eg: Ethernet) to ensure we have support for them.
   INF Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf
 
+  # SCMI Driver
+  INF ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
+
 !if $(ARCH) == AARCH64
   #
   # EBC
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
index d3fc9b6cc7fbed8bd9591a4a57f52e966b99534a..90628e04b27ad4546d9150309d782b91ff40b068 100644
--- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2013-2016, ARM Limited. All rights reserved.
+#  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -54,6 +54,9 @@ [FixedPcd]
   gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
   gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
 
+  # Framebuffer Memory
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
 
   #
   # PL011 Serial Debug UART
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
index 2cac815e96104e74c9ceae2c4140bcab535432a8..2403f51fa94b16bacb921a9695a40e1b5068c7cf 100644
--- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
@@ -21,8 +21,10 @@
 
 #include <ArmPlatform.h>
 
+#define FRAME_BUFFER_DESCRIPTOR ((FixedPcdGet32 (PcdArmLcdDdrFrameBufferBase) != 0) ? 1 : 0)
+
 // The total number of descriptors, including the final "end-of-table" descriptor.
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 16
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (16 + FRAME_BUFFER_DESCRIPTOR)
 
 // DDR attributes
 #define DDR_ATTRIBUTES_CACHED           ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
@@ -149,6 +151,23 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[Index].Length          = ARM_JUNO_SOC_PERIPHERALS_SZ;
   VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
+  // Framebuffer Memory
+  if (FixedPcdGet32 (PcdArmLcdDdrFrameBufferBase) != 0) {
+    ASSERT ((PcdGet64 (PcdSystemMemoryBase) +
+             PcdGet64 (PcdSystemMemorySize) - 1) <
+             FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase));
+    VirtualMemoryTable[++Index].PhysicalBase  = FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
+    VirtualMemoryTable[Index].VirtualBase     = FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
+    VirtualMemoryTable[Index].Length          = FixedPcdGet32 (PcdArmLcdDdrFrameBufferSize);
+    // Map as Normal Non-Cacheable memory, so that we can use the accelerated
+    // SetMem/CopyMem routines that may use unaligned accesses or
+    // DC ZVA instructions. If mapped as device memory, these routine may cause
+    // alignment faults.
+    // NOTE: The attribute value is misleading, it indicates memory map type as
+    // an un-cached, un-buffered but allows buffering and reordering.
+    VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
+  }
+
   // DDR - 2GB
   VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdSystemMemoryBase);
   VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdSystemMemoryBase);
diff --git a/Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c b/Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
new file mode 100644
index 0000000000000000000000000000000000000000..db3871af0ac6bc8c21489713eb44699b848cc246
--- /dev/null
+++ b/Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
@@ -0,0 +1,555 @@
+/** @file
+
+  Copyright (c) 2013-2018, ARM Ltd. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiDxe.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/LcdPlatformLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/ArmScmi.h>
+#include <Protocol/ArmScmiClockProtocol.h>
+#include <Protocol/Cpu.h>
+
+/* Display timings on Juno for 1920x1080.
+  On Juno due to instability of the PLLs, we set OSC
+  frequency to 138.5MHz which is stable for most monitors.
+  Frequency 148.5MHz does not work with some monitors.
+  148.5MHz is set by SCP firmware by default.
+
+#define JUNO_HD_OSC_FREQUENCY               148500000
+*/
+#define JUNO_HD_OSC_FREQUENCY               138500000
+#define JUNO_HD_H_SYNC                      ( 32 - 1)
+#define JUNO_HD_H_FRONT_PORCH               ( 48 - 1)
+#define JUNO_HD_H_BACK_PORCH                ( 80 - 1)
+#define JUNO_HD_V_SYNC                      (  5 - 1)
+#define JUNO_HD_V_FRONT_PORCH               (  3 - 1)
+#define JUNO_HD_V_BACK_PORCH                ( 23 - 1)
+
+/* SCMI defined clock device name and ID. This is not documented but
+   obtained using clock management protocol's CLOCK_ATTRIBUTES command.
+
+   Generally we must discover clock device ID using clock name and then
+   set/get rate using CLOCK_RATE_SET/CLOCK_RATE_GET commands. However
+   because LcdGraphicsOutputDxe is a DXE driver, which gets initialized
+   at boot time, for faster boot, in release build we will directly use
+   this already known value as an argument to rate get/set functions.
+
+   We expect these values not to change in future SCP firmware releases.
+
+   DEBUG build however will probe SCP firmware and discover clock device
+   ID for HDLCD.
+*/
+#define ARM_JUNO_CSS_CLK_NAME_HDLCD_0       "HDLCD_0"
+#define ARM_JUNO_CSS_CLKID_HDLCD_0           3
+
+typedef struct {
+  UINT32                      OscFreq;
+  SCAN_TIMINGS                Horizontal;
+  SCAN_TIMINGS                Vertical;
+} DISPLAY_MODE;
+
+STATIC DISPLAY_MODE mDisplayModes[] = {
+  {
+    // VGA : 640 x 480 x 24 bpp.
+    VGA_OSC_FREQUENCY,
+    {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
+    {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
+  },
+  {
+    // WVGA : 800 x 480 x 24 bpp.
+    WVGA_OSC_FREQUENCY,
+    {WVGA_H_RES_PIXELS, WVGA_H_SYNC, WVGA_H_BACK_PORCH, WVGA_H_FRONT_PORCH},
+    {WVGA_V_RES_PIXELS, WVGA_V_SYNC, WVGA_V_BACK_PORCH, WVGA_V_FRONT_PORCH}
+  },
+  {
+    // SVGA : 800 x 600 x 24 bpp.
+    SVGA_OSC_FREQUENCY,
+    {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
+    {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
+  },
+  {
+    // QHD : 960 x 540 x 24 bpp.
+    QHD_OSC_FREQUENCY,
+    {QHD_H_RES_PIXELS, QHD_H_SYNC, QHD_H_BACK_PORCH, QHD_H_FRONT_PORCH},
+    {QHD_V_RES_PIXELS, QHD_V_SYNC, QHD_V_BACK_PORCH, QHD_V_FRONT_PORCH}
+  },
+  {
+    // WSVGA : 1024 x 600 x 24 bpp.
+    WSVGA_OSC_FREQUENCY,
+    {WSVGA_H_RES_PIXELS, WSVGA_H_SYNC, WSVGA_H_BACK_PORCH, WSVGA_H_FRONT_PORCH},
+    {WSVGA_V_RES_PIXELS, WSVGA_V_SYNC, WSVGA_V_BACK_PORCH, WSVGA_V_FRONT_PORCH}
+  },
+  {
+    // XGA : 1024 x 768 x 24 bpp.
+    XGA_OSC_FREQUENCY,
+    {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
+    {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
+  },
+  {
+    // HD : 1280 x 720 x 24 bpp.
+    HD720_OSC_FREQUENCY,
+    {HD720_H_RES_PIXELS, HD720_H_SYNC, HD720_H_BACK_PORCH, HD720_H_FRONT_PORCH},
+    {HD720_V_RES_PIXELS, HD720_V_SYNC, HD720_V_BACK_PORCH, HD720_V_FRONT_PORCH}
+  },
+  {
+    // WXGA : 1280 x 800 x 24 bpp.
+    WXGA_OSC_FREQUENCY,
+    {WXGA_H_RES_PIXELS, WXGA_H_SYNC, WXGA_H_BACK_PORCH, WXGA_H_FRONT_PORCH},
+    {WXGA_V_RES_PIXELS, WXGA_V_SYNC, WXGA_V_BACK_PORCH, WXGA_V_FRONT_PORCH}
+  },
+  {
+    // SXGA : 1280 x 1024 x 24 bpp.
+    SXGA_OSC_FREQUENCY,
+    {SXGA_H_RES_PIXELS, SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH},
+    {SXGA_V_RES_PIXELS, SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH}
+  },
+  {
+    // WSXGA+ : 1680 x 1050 x 24 bpp.
+    WSXGA_OSC_FREQUENCY,
+    {WSXGA_H_RES_PIXELS, WSXGA_H_SYNC, WSXGA_H_BACK_PORCH, WSXGA_H_FRONT_PORCH},
+    {WSXGA_V_RES_PIXELS, WSXGA_V_SYNC, WSXGA_V_BACK_PORCH, WSXGA_V_FRONT_PORCH}
+  },
+  {
+    // HD : 1920 x 1080 x 24 bpp.
+    JUNO_HD_OSC_FREQUENCY,
+    {HD_H_RES_PIXELS, JUNO_HD_H_SYNC, JUNO_HD_H_BACK_PORCH, JUNO_HD_H_FRONT_PORCH},
+    {HD_V_RES_PIXELS, JUNO_HD_V_SYNC, JUNO_HD_V_BACK_PORCH, JUNO_HD_V_FRONT_PORCH}
+  }
+};
+
+/* If PcdArmMaliDpMaxMode is 0, platform supports full range of modes
+   else platform supports modes from 0 to PcdArmHdLcdMaxMode - 1
+*/
+STATIC CONST UINT32 mMaxMode = ((FixedPcdGet32 (PcdArmHdLcdMaxMode) != 0)
+                                   ? FixedPcdGet32 (PcdArmHdLcdMaxMode)
+                                   : sizeof (mDisplayModes) / sizeof (DISPLAY_MODE));
+
+/** HDLCD 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
+LcdPlatformInitializeDisplay (
+  IN  EFI_HANDLE  Handle
+  )
+{
+  (VOID)Handle;
+  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;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/** Allocate VRAM memory in DRAM for the framebuffer
+  (unless it is reserved already).
+
+  The allocated address can be used to set the framebuffer.
+
+  @param[out] VramBaseAddress     A pointer to the framebuffer address.
+  @param[out] VramSize            A pointer to the size of the framebuffer
+                                  in bytes
+
+  @retval EFI_SUCCESS             Framebuffer memory allocated successfully.
+  @retval !(EFI_SUCCESS)          Other errors.
+**/
+EFI_STATUS
+LcdPlatformGetVram (
+  OUT EFI_PHYSICAL_ADDRESS  * VramBaseAddress,
+  OUT UINTN                 * VramSize
+  )
+{
+  EFI_STATUS      Status = EFI_SUCCESS;
+
+  EFI_CPU_ARCH_PROTOCOL  *Cpu;
+
+  ASSERT (VramBaseAddress != NULL);
+  ASSERT (VramSize != NULL);
+
+  // Set the VRAM size.
+  *VramSize = (UINTN)FixedPcdGet32 (PcdArmLcdDdrFrameBufferSize);
+
+  // Check if memory is already reserved for the framebuffer.
+  *VramBaseAddress =
+     (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
+
+  if (*VramBaseAddress == 0) {
+    // If not already reserved, attempt to allocate the VRAM from the DRAM.
+    Status = gBS->AllocatePages (
+                    AllocateAnyPages,
+                    EfiRuntimeServicesData,
+                    EFI_SIZE_TO_PAGES (*VramSize),
+                    VramBaseAddress
+                    );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "HdLcdArmJuno: Failed to allocate frame buffer.\n"));
+      ASSERT_EFI_ERROR (Status);
+      return Status;
+    }
+
+    // Ensure the Cpu architectural protocol is already installed
+    Status = gBS->LocateProtocol (
+                    &gEfiCpuArchProtocolGuid,
+                    NULL,
+                    (VOID **)&Cpu
+                    );
+    if (!EFI_ERROR (Status)) {
+      // The VRAM is inside the DRAM, which is cacheable.
+      // Mark the VRAM as write-combining (uncached) and non-executable.
+      Status = Cpu->SetMemoryAttributes (
+                      Cpu,
+                      *VramBaseAddress,
+                      *VramSize,
+                      EFI_MEMORY_WC | EFI_MEMORY_XP
+                      );
+    }
+    if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
+      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
+    }
+  }
+
+  return Status;
+}
+
+/** Return total number of modes supported.
+
+  Note: Valid mode numbers are 0 to MaxMode - 1
+  See Section 12.9 of the UEFI Specification 2.7
+
+  @retval UINT32             Mode Number.
+**/
+UINT32
+LcdPlatformGetMaxMode (VOID)
+{
+  return  mMaxMode;
+}
+
+#if !defined(MDEPKG_NDEBUG)
+/** Probe Clock device ID of the HDLCD clock and current pixel clock frequency.
+  NOTE: We will probe information only in DEBUG build.
+
+  @param[in]  ClockProtocol   A pointer to SCMI clock protocol
+                              interface instance.
+  @param[out] ClockId         ID of the clock device
+
+  @retval EFI_SUCCESS         Clock ID of the HDLCD device returned
+                              successfully.
+  @retval EFI_UNSUPPORTED     SCMI clock management protocol unsupported.
+  @retval EFI_DEVICE_ERROR    SCMI error.
+  @retval EFI_NOT_FOUND       Not found valid clock device ID of the HDLCD.
+**/
+STATIC
+EFI_STATUS
+ProbeHdLcdClock (
+  IN  SCMI_CLOCK_PROTOCOL  * ClockProtocol,
+  OUT UINT32               * ClockId
+  )
+{
+  EFI_STATUS  Status;
+  UINT64      CurrentHdLcdFreq;
+
+  UINT32      TotalClocks;
+  UINT32      ClockProtocolVersion;
+  BOOLEAN     Enabled;
+  CHAR8       ClockName[SCMI_MAX_STR_LEN];
+  BOOLEAN     ClockFound = FALSE;
+
+  UINT32                 TotalRates = 0;
+  UINT32                 ClockRateSize;
+  SCMI_CLOCK_RATE        ClockRate;
+  SCMI_CLOCK_RATE_FORMAT ClockRateFormat;
+
+  Status = ClockProtocol->GetVersion (ClockProtocol, &ClockProtocolVersion);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  DEBUG ((DEBUG_ERROR, "SCMI clock management protocol version = %x\n",
+    ClockProtocolVersion));
+
+  if (ClockProtocolVersion != SCMI_CLOCK_PROTOCOL_VERSION) {
+    ASSERT (FALSE);
+    return EFI_UNSUPPORTED;
+  }
+
+  Status = ClockProtocol->GetTotalClocks (ClockProtocol, &TotalClocks);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  DEBUG ((DEBUG_ERROR, "Total number of clocks supported by SCMI clock management protocol = %d\n",
+    TotalClocks));
+
+  for (*ClockId = 0; *ClockId < TotalClocks; (*ClockId)++) {
+    Status = ClockProtocol->GetClockAttributes (
+                              ClockProtocol,
+                              *ClockId,
+                              &Enabled,
+                              ClockName
+                              );
+    if (EFI_ERROR (Status)) {
+      // In current implementation of SCMI, some clocks are not accessible to
+      // calling agents (in our case UEFI is an agent) which results in an
+      // EFI_DEVICE_ERROR error. A bug fix for this is in discussions and will
+      // be fixed in future versions of the SCP firmware. Irrespective of a fix
+      // we must iterate over each clock to see if it matches with HDLCD.
+      continue;
+    }
+
+    if (AsciiStrnCmp ((CONST CHAR8*)ClockName,
+          (CONST CHAR8*)ARM_JUNO_CSS_CLK_NAME_HDLCD_0,
+          sizeof (ARM_JUNO_CSS_CLK_NAME_HDLCD_0)) == 0) {
+      ClockFound = TRUE;
+      break;
+    }
+  }
+
+  if (!ClockFound) {
+    return EFI_NOT_FOUND;
+  }
+
+  ClockRateSize = sizeof (ClockRate);
+  Status = ClockProtocol->DescribeRates (
+                            ClockProtocol,
+                            *ClockId,
+                            &ClockRateFormat,
+                            &TotalRates,
+                            &ClockRateSize,
+                            &ClockRate
+                            );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = ClockProtocol->RateGet (ClockProtocol, *ClockId, &CurrentHdLcdFreq);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  DEBUG ((DEBUG_ERROR, "Clock ID = %d Clock name = %a\n", *ClockId, ClockName));
+  DEBUG ((DEBUG_ERROR, "Minimum frequency = %uHz\n", ClockRate.Min));
+  DEBUG ((DEBUG_ERROR, "Maximum frequency = %uHz\n", ClockRate.Max));
+  DEBUG ((DEBUG_ERROR, "Clock rate step = %uHz\n", ClockRate.Step));
+
+  DEBUG ((DEBUG_ERROR, "HDLCD Current frequency = %uHz\n", CurrentHdLcdFreq));
+
+  return EFI_SUCCESS;
+}
+#endif
+
+/** Set the requested display mode.
+
+  @param[in] ModeNumber             Mode Number.
+
+  @retval EFI_SUCCESS              Mode set successfully.
+  @retval EFI_NOT_FOUND            Clock protocol instance not found.
+  @retval EFI_DEVICE_ERROR         SCMI error.
+  @retval EFI_INVALID_PARAMETER    Requested mode not found.
+  @retval !(EFI_SUCCESS)           Other errors.
+*/
+EFI_STATUS
+LcdPlatformSetMode (
+  IN  UINT32  ModeNumber
+  )
+{
+  EFI_STATUS          Status;
+  SCMI_CLOCK_PROTOCOL *ClockProtocol;
+  UINT32              ClockId;
+
+  EFI_GUID ClockProtocolGuid = ARM_SCMI_CLOCK_PROTOCOL_GUID;
+
+  if (ModeNumber >= mMaxMode) {
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Display debug information in boot log.
+  DEBUG ((DEBUG_ERROR, "HDLCD Display controller:\n"));
+
+  DEBUG ((DEBUG_ERROR, "Required frequency for resolution %dx%d = %uHz\n",
+            mDisplayModes[ModeNumber].Horizontal.Resolution,
+            mDisplayModes[ModeNumber].Vertical.Resolution,
+            mDisplayModes[ModeNumber].OscFreq));
+
+  Status = gBS->LocateProtocol (
+                  &ClockProtocolGuid,
+                  NULL,
+                  (VOID**)&ClockProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+#if !defined(MDEPKG_NDEBUG)
+  /* Avoid probing clock device ID in RELEASE build */
+  Status = ProbeHdLcdClock (ClockProtocol, &ClockId);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  ASSERT (ClockId == ARM_JUNO_CSS_CLKID_HDLCD_0);
+#else
+  ClockId = ARM_JUNO_CSS_CLKID_HDLCD_0;
+#endif
+
+  // Set HDLCD clock required for the requested mode
+  Status = ClockProtocol->RateSet (
+                            ClockProtocol,
+                            ClockId,
+                            mDisplayModes[ModeNumber].OscFreq
+                            );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "SCMI error: %r\n", Status));
+    return Status;
+  }
+
+#if !defined(MDEPKG_NDEBUG)
+  UINT64  CurrentHdLcdFreq;
+  // Actual value set can differ from requested frequency so verify.
+  Status = ClockProtocol->RateGet (
+                            ClockProtocol,
+                            ARM_JUNO_CSS_CLKID_HDLCD_0,
+                            &CurrentHdLcdFreq
+                            );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "SCMI Error: %r\n", Status));
+  } else {
+    DEBUG ((DEBUG_ERROR, "Requested frequency change = %uHz, Actual changed frequency = %uHz\n",
+      mDisplayModes[ModeNumber].OscFreq,
+      CurrentHdLcdFreq
+      ));
+  }
+#endif
+
+  return Status;
+}
+
+/** Return information for the requested mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] Info                Pointer for returned mode information
+                                  (on success).
+
+  @retval EFI_SUCCESS             Mode information for the requested mode
+                                  returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
+EFI_STATUS
+LcdPlatformQueryMode (
+  IN  UINT32                                  ModeNumber,
+  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  * Info
+  )
+{
+  if (ModeNumber >= mMaxMode ){
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ASSERT (Info != NULL);
+
+  Info->Version = 0;
+  Info->HorizontalResolution = mDisplayModes[ModeNumber].Horizontal.Resolution;
+  Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
+  Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
+
+  Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
+
+  return EFI_SUCCESS;
+}
+
+/** Return display timing information for the requested mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] Horizontal          Pointer to horizontal timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
+  @param[out] Vertical            Pointer to vertical timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
+
+  @retval EFI_SUCCESS             Display timing information for the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
+EFI_STATUS
+LcdPlatformGetTimings (
+  IN  UINT32           ModeNumber,
+  OUT SCAN_TIMINGS  ** Horizontal,
+  OUT SCAN_TIMINGS  ** Vertical
+  )
+{
+  if (ModeNumber >= mMaxMode ){
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ASSERT (Horizontal != NULL);
+  ASSERT (Vertical != NULL);
+
+  *Horizontal = &mDisplayModes[ModeNumber].Horizontal;
+  *Vertical   = &mDisplayModes[ModeNumber].Vertical;
+
+  return EFI_SUCCESS;
+}
+
+/** Return bits per pixel information for a mode number.
+
+  @param[in]  ModeNumber          Mode Number.
+
+  @param[out] Bpp                 Pointer to bits per pixel information.
+
+  @retval EFI_SUCCESS             Bits per pixel information for the requested
+                                  mode returned successfully.
+  @retval EFI_INVALID_PARAMETER   Requested mode not found.
+**/
+EFI_STATUS
+LcdPlatformGetBpp (
+  IN  UINT32     ModeNumber,
+  OUT LCD_BPP  * Bpp
+  )
+{
+  if (ModeNumber >= mMaxMode) {
+    // Check valid ModeNumber and Bpp.
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ASSERT (Bpp != NULL);
+
+  *Bpp = LCD_BITS_PER_PIXEL_24;
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf b/Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..5590d95e2db15b2dea883d25572ae73efcf8f10b
--- /dev/null
+++ b/Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
@@ -0,0 +1,41 @@
+#/** @file
+#
+#  Component description file for HdLcdArmJunoLib module
+#
+#  Copyright (c) 2013-2018, ARM Ltd. All rights reserved.<BR>
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010019
+  BASE_NAME                      = HdLcdArmJunoLib
+  FILE_GUID                      = 7B1D26F7-7B88-47ED-B193-DD3BDF319006
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = LcdPlatformLib
+
+[Sources.common]
+  HdLcdArmJuno.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/JunoPkg/ArmJuno.dec
+
+[LibraryClasses]
+  BaseLib
+
+[FixedPcd]
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
+  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
+  gArmJunoTokenSpaceGuid.PcdArmHdLcdMaxMode
+  gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'




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

* Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData Girish Pathak
@ 2018-03-21  3:37   ` Ard Biesheuvel
  2018-03-21 11:07     ` Girish Pathak
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2018-03-21  3:37 UTC (permalink / raw)
  To: Girish Pathak
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Matteo Carlini,
	Stephanie.Hughes-Fitt, nd, Arvind Chauhan, Daniil Egranov,
	Thomas Panakamattam Abraham

On 21 March 2018 at 00:18, Girish Pathak <girish.pathak@arm.com> wrote:
> As per the UEFI specification(2.7) section 12.9, the GOP framebuffer
> memory can be accessed in the pre-boot and the post boot phase (by OS)
> Therefore the memory type EfiBootServicesData is incorrect for
> the framebuffer memory allocation. Change EfiBootServicesData with
> EfiRuntimeServicesData flag so that allocated memory can be access
> by the OS in the post boot phase.
>

EfiRuntimeServicesData is intended for allocations that the EFI
runtime services need to access themselves at runtime, and will hence
be virtually remapped by SetVirtualAddressMap().

This does not apply to the framebuffer. Even if it may be used at OS
runtime, the firmware itself will never access it, so
EfiRuntimeServicesData is not appropriate

Please use EfiReservedMemory instead.


> 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/HdLcdArmVExpress.c       | 2 +-
>  Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> index f7cae39c9cc9954ba4cad1bd597ebfc8baf10f11..c0a25a18d3fcfe91a76ee985ee58145b97900fa0 100644
> --- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> +++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> @@ -176,7 +176,7 @@ LcdPlatformGetVram (
>    }
>    Status = gBS->AllocatePages (
>                    AllocationType,
> -                  EfiBootServicesData,
> +                  EfiRuntimeServicesData,
>                    EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
>                    VramBaseAddress
>                    );
> diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 2f4814a2adbf01517ba14d75ce579ff35c362379..61ddf77e903e6c33a26b2aa8b76121e807195a9a 100644
> --- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -232,7 +232,7 @@ LcdPlatformGetVram (
>      // Allocate the VRAM from the DRAM so that nobody else uses it.
>      Status = gBS->AllocatePages (
>                      AllocateAddress,
> -                    EfiBootServicesData,
> +                    EfiRuntimeServicesData,
>                      EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
>                      VramBaseAddress
>                      );
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>


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

* Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
  2018-03-21  3:37   ` Ard Biesheuvel
@ 2018-03-21 11:07     ` Girish Pathak
  2018-03-21 18:26       ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Girish Pathak @ 2018-03-21 11:07 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Matteo Carlini,
	Stephanie Hughes-Fitt, nd, Arvind Chauhan, Daniil Egranov,
	Thomas Abraham



> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 21 March 2018 03:38
> To: Girish Pathak <Girish.Pathak@arm.com>
> Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>;
> Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt
> <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>; Arvind Chauhan
> <Arvind.Chauhan@arm.com>; Daniil Egranov <Daniil.Egranov@arm.com>;
> Thomas Abraham <thomas.abraham@arm.com>
> Subject: Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate
> framebuffer using EfiRuntimeServicesData
> 
> On 21 March 2018 at 00:18, Girish Pathak <girish.pathak@arm.com> wrote:
> > As per the UEFI specification(2.7) section 12.9, the GOP framebuffer
> > memory can be accessed in the pre-boot and the post boot phase (by OS)
> > Therefore the memory type EfiBootServicesData is incorrect for the
> > framebuffer memory allocation. Change EfiBootServicesData with
> > EfiRuntimeServicesData flag so that allocated memory can be access by
> > the OS in the post boot phase.
> >
> 
> EfiRuntimeServicesData is intended for allocations that the EFI runtime
> services need to access themselves at runtime, and will hence be virtually
> remapped by SetVirtualAddressMap().
> 
> This does not apply to the framebuffer. Even if it may be used at OS runtime,
> the firmware itself will never access it, so EfiRuntimeServicesData is not
> appropriate
> 
> Please use EfiReservedMemory instead.

Specification (UEFI Spec 2_7_A Sept 6.pdf) describes EfiReservedMemoryType as  Not usable before and after ExitBootServices, See Table 28 & 29
Hence EfiReservedMemoryType is not suitable in this case.  Agree? 

> 
> > 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/HdLcdArmVExpr
> ess.c       | 2 +-
> >
> >
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArm
> VEx
> > press.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> >
> a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> pres
> > s.c
> >
> b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> pres
> > s.c index
> >
> f7cae39c9cc9954ba4cad1bd597ebfc8baf10f11..c0a25a18d3fcfe91a76ee985ee
> 58
> > 145b97900fa0 100644
> > ---
> >
> a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> pres
> > s.c
> > +++
> b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> > +++ press.c
> > @@ -176,7 +176,7 @@ LcdPlatformGetVram (
> >    }
> >    Status = gBS->AllocatePages (
> >                    AllocationType,
> > -                  EfiBootServicesData,
> > +                  EfiRuntimeServicesData,
> >                    EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
> >                    VramBaseAddress
> >                    );
> > diff --git
> >
> a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mV
> > Express.c
> >
> b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mV
> > Express.c index
> >
> 2f4814a2adbf01517ba14d75ce579ff35c362379..61ddf77e903e6c33a26b2aa8b7
> 61
> > 21e807195a9a 100644
> > ---
> >
> a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mV
> > Express.c
> > +++
> b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111Lcd
> > +++ ArmVExpress.c
> > @@ -232,7 +232,7 @@ LcdPlatformGetVram (
> >      // Allocate the VRAM from the DRAM so that nobody else uses it.
> >      Status = gBS->AllocatePages (
> >                      AllocateAddress,
> > -                    EfiBootServicesData,
> > +                    EfiRuntimeServicesData,
> >                      EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
> >                      VramBaseAddress
> >                      );
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
> >

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

* Re: [PATCH edk2-platforms v3 00/17] Update GOP
  2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
                   ` (16 preceding siblings ...)
  2018-03-20 16:18 ` [PATCH edk2-platforms v3 17/17] ARM/JunoPkg: Add HDLCD platform library Girish Pathak
@ 2018-03-21 12:56 ` Evan Lloyd
  17 siblings, 0 replies; 24+ messages in thread
From: Evan Lloyd @ 2018-03-21 12:56 UTC (permalink / raw)
  To: Girish Pathak, edk2-devel@lists.01.org
  Cc: nd, ard.biesheuvel@linaro.org, leif.lindholm@linaro.org,
	Stephanie Hughes-Fitt, Arvind Chauhan

To minimise the spam involved, I'm hoping the maintainers will accept this as covering all the patches listed.  (Please?)

Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>


> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:18
> To: edk2-devel@lists.01.org
> Cc: nd <nd@arm.com>; ard.biesheuvel@linaro.org;
> leif.lindholm@linaro.org; Stephanie Hughes-Fitt <Stephanie.Hughes-
> Fitt@arm.com>; Arvind Chauhan <Arvind.Chauhan@arm.com>
> Subject: [edk2] [PATCH edk2-platforms v3 00/17] Update GOP
> 
> This patch series addresses comments on the patch v2
> (https://lists.01.org/pipermail/edk2-devel/2017-December/019406.html)
> reworking of the Graphics Output Protocol code in ArmPlatformPkg.
> It also contains updates for the new SCMI protocol (MTL Library).
> 
> Code is available for examination at:
>   https://github.com/girishpathak/edk2-platforms/tree/201_gop_v3
> 
> 
> Ard Biesheuvel (1):
>   ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries
> 
> EvanLloyd (2):
>   ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp
>   ARM/VExpressPkg: Redefine LcdPlatformGetTimings function
> 
> Girish Pathak (14):
>   ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding
> standard
>   ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments
>   ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD
> inf
>   ARM/VExpressPkg: Add and update debug ASSERTS
>   ARM/VExpressPkg: PL111Lcd/HdLcd plaform libs: Minor code cleanup
>   ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32
>   ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check
> EFI_TIMEOUT
>   ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format
>   ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
>   ARM/VExpressPkg: Reserving framebuffer at build
>   ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer
>   ARM/VExpressPkg: New DP500/DP550/DP650 platform library
>   ARM/JunoPkg: Adding SCMI MTL library
>   ARM/JunoPkg: Add HDLCD platform library
> 
>  Platform/ARM/JunoPkg/ArmJuno.dec                                                   |  17 +-
>  Platform/ARM/JunoPkg/ArmJuno.dsc                                                   |  31 +-
>  Platform/ARM/JunoPkg/ArmJuno.fdf                                                   |  12 +-
>  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> |   5 +-
>  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> |  29 +-
>  Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c
> | 198 +++++++
>  Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
> |  39 ++
>  Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h
> |  94 ++++
>  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
> | 555 ++++++++++++++++++++
>  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
> |  41 ++
>  Platform/ARM/VExpressPkg/ArmVExpressPkg.dec                                        |
> 3 +-
>  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
> | 387 ++++++++++++++
>  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> |  43 ++
> 
> Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib
> .inf             |   7 +-
>  Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> |  53 +-
> 
> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExp
> ress.c            | 310 +++++++----
> 
> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExp
> ressLib.inf       |  14 +-
> 
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mVExpress.c      | 389 +++++++++-----
> 
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mVExpressLib.inf |  10 +-
>  19 files changed, 1945 insertions(+), 292 deletions(-)  create mode 100644
> Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c
>  create mode 100644
> Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
>  create mode 100644
> Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h
>  create mode 100644
> Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
>  create mode 100644
> Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
>  create mode 100644
> Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
>  create mode 100644
> Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
  2018-03-21 11:07     ` Girish Pathak
@ 2018-03-21 18:26       ` Ard Biesheuvel
  2018-03-22 15:20         ` Evan Lloyd
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2018-03-21 18:26 UTC (permalink / raw)
  To: Girish Pathak
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Matteo Carlini,
	Stephanie Hughes-Fitt, nd, Arvind Chauhan, Daniil Egranov,
	Thomas Abraham

On 21 March 2018 at 19:07, Girish Pathak <Girish.Pathak@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 21 March 2018 03:38
>> To: Girish Pathak <Girish.Pathak@arm.com>
>> Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>;
>> Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt
>> <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>; Arvind Chauhan
>> <Arvind.Chauhan@arm.com>; Daniil Egranov <Daniil.Egranov@arm.com>;
>> Thomas Abraham <thomas.abraham@arm.com>
>> Subject: Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate
>> framebuffer using EfiRuntimeServicesData
>>
>> On 21 March 2018 at 00:18, Girish Pathak <girish.pathak@arm.com> wrote:
>> > As per the UEFI specification(2.7) section 12.9, the GOP framebuffer
>> > memory can be accessed in the pre-boot and the post boot phase (by OS)
>> > Therefore the memory type EfiBootServicesData is incorrect for the
>> > framebuffer memory allocation. Change EfiBootServicesData with
>> > EfiRuntimeServicesData flag so that allocated memory can be access by
>> > the OS in the post boot phase.
>> >
>>
>> EfiRuntimeServicesData is intended for allocations that the EFI runtime
>> services need to access themselves at runtime, and will hence be virtually
>> remapped by SetVirtualAddressMap().
>>
>> This does not apply to the framebuffer. Even if it may be used at OS runtime,
>> the firmware itself will never access it, so EfiRuntimeServicesData is not
>> appropriate
>>
>> Please use EfiReservedMemory instead.
>
> Specification (UEFI Spec 2_7_A Sept 6.pdf) describes EfiReservedMemoryType as  Not usable before and after ExitBootServices, See Table 28 & 29
> Hence EfiReservedMemoryType is not suitable in this case.  Agree?
>

It is not usable as ordinary memory, given that you turn it into
'special' memory (with side effects) by turning it into a framebuffer.

So EfiReservedMemory is perfectly appropriate here.


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

* Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
  2018-03-21 18:26       ` Ard Biesheuvel
@ 2018-03-22 15:20         ` Evan Lloyd
  2018-03-22 17:38           ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Evan Lloyd @ 2018-03-22 15:20 UTC (permalink / raw)
  To: Ard Biesheuvel, Girish Pathak
  Cc: nd, edk2-devel@lists.01.org, Leif Lindholm, Stephanie Hughes-Fitt,
	Arvind Chauhan

Hi Ard.

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Ard
> Biesheuvel
> Sent: 21 March 2018 18:27
> To: Girish Pathak <Girish.Pathak@arm.com>
> Cc: nd <nd@arm.com>; edk2-devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Stephanie Hughes-Fitt <Stephanie.Hughes-
> Fitt@arm.com>; Arvind Chauhan <Arvind.Chauhan@arm.com>
> Subject: Re: [edk2] [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg:
> Allocate framebuffer using EfiRuntimeServicesData
> 
> On 21 March 2018 at 19:07, Girish Pathak <Girish.Pathak@arm.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: 21 March 2018 03:38
> >> To: Girish Pathak <Girish.Pathak@arm.com>
> >> Cc: edk2-devel@lists.01.org; Leif Lindholm
> >> <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>;
> >> Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd
> >> <nd@arm.com>; Arvind Chauhan <Arvind.Chauhan@arm.com>; Daniil
> Egranov
> >> <Daniil.Egranov@arm.com>; Thomas Abraham
> <thomas.abraham@arm.com>
> >> Subject: Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg:
> >> Allocate framebuffer using EfiRuntimeServicesData
> >>
> >> On 21 March 2018 at 00:18, Girish Pathak <girish.pathak@arm.com>
> wrote:
> >> > As per the UEFI specification(2.7) section 12.9, the GOP
> >> > framebuffer memory can be accessed in the pre-boot and the post
> >> > boot phase (by OS) Therefore the memory type EfiBootServicesData is
> >> > incorrect for the framebuffer memory allocation. Change
> >> > EfiBootServicesData with EfiRuntimeServicesData flag so that
> >> > allocated memory can be access by the OS in the post boot phase.
> >> >
> >>
> >> EfiRuntimeServicesData is intended for allocations that the EFI
> >> runtime services need to access themselves at runtime, and will hence
> >> be virtually remapped by SetVirtualAddressMap().
> >>
> >> This does not apply to the framebuffer. Even if it may be used at OS
> >> runtime, the firmware itself will never access it, so
> >> EfiRuntimeServicesData is not appropriate
> >>
> >> Please use EfiReservedMemory instead.
> >
> > Specification (UEFI Spec 2_7_A Sept 6.pdf) describes
> > EfiReservedMemoryType as  Not usable before and after ExitBootServices,
> See Table 28 & 29 Hence EfiReservedMemoryType is not suitable in this
> case.  Agree?
> >
> 
> It is not usable as ordinary memory, given that you turn it into 'special'
> memory (with side effects) by turning it into a framebuffer.
> 
> So EfiReservedMemory is perfectly appropriate here.
[[Evan Lloyd]] First, I agree EfiReservedMemory is probably the sensible option.  The only alternative would be EfiMemoryMappedIO, and that, as you have pointed out in the past, introduces alignment requirements.  If you are happy to accept it, I'll ask Girish to go with that.  However, Girish has a point, if only that the UEFI spec need more clarity on this, especially as https://lists.01.org/pipermail/edk2-devel/2017-February/007494.html pretty much confirms it is the right way to go.  In particular, the bald statement "EfiReservedMemoryType    Not usable.", seems unfortunate.

Regards,
Evan

> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
  2018-03-22 15:20         ` Evan Lloyd
@ 2018-03-22 17:38           ` Ard Biesheuvel
  0 siblings, 0 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2018-03-22 17:38 UTC (permalink / raw)
  To: Evan Lloyd
  Cc: Girish Pathak, nd, edk2-devel@lists.01.org, Leif Lindholm,
	Stephanie Hughes-Fitt, Arvind Chauhan

On 22 March 2018 at 15:20, Evan Lloyd <Evan.Lloyd@arm.com> wrote:
> Hi Ard.
>
>> -----Original Message-----
>> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Ard
>> Biesheuvel
>> Sent: 21 March 2018 18:27
>> To: Girish Pathak <Girish.Pathak@arm.com>
>> Cc: nd <nd@arm.com>; edk2-devel@lists.01.org; Leif Lindholm
>> <leif.lindholm@linaro.org>; Stephanie Hughes-Fitt <Stephanie.Hughes-
>> Fitt@arm.com>; Arvind Chauhan <Arvind.Chauhan@arm.com>
>> Subject: Re: [edk2] [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg:
>> Allocate framebuffer using EfiRuntimeServicesData
>>
>> On 21 March 2018 at 19:07, Girish Pathak <Girish.Pathak@arm.com>
>> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> >> Sent: 21 March 2018 03:38
>> >> To: Girish Pathak <Girish.Pathak@arm.com>
>> >> Cc: edk2-devel@lists.01.org; Leif Lindholm
>> >> <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>;
>> >> Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd
>> >> <nd@arm.com>; Arvind Chauhan <Arvind.Chauhan@arm.com>; Daniil
>> Egranov
>> >> <Daniil.Egranov@arm.com>; Thomas Abraham
>> <thomas.abraham@arm.com>
>> >> Subject: Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg:
>> >> Allocate framebuffer using EfiRuntimeServicesData
>> >>
>> >> On 21 March 2018 at 00:18, Girish Pathak <girish.pathak@arm.com>
>> wrote:
>> >> > As per the UEFI specification(2.7) section 12.9, the GOP
>> >> > framebuffer memory can be accessed in the pre-boot and the post
>> >> > boot phase (by OS) Therefore the memory type EfiBootServicesData is
>> >> > incorrect for the framebuffer memory allocation. Change
>> >> > EfiBootServicesData with EfiRuntimeServicesData flag so that
>> >> > allocated memory can be access by the OS in the post boot phase.
>> >> >
>> >>
>> >> EfiRuntimeServicesData is intended for allocations that the EFI
>> >> runtime services need to access themselves at runtime, and will hence
>> >> be virtually remapped by SetVirtualAddressMap().
>> >>
>> >> This does not apply to the framebuffer. Even if it may be used at OS
>> >> runtime, the firmware itself will never access it, so
>> >> EfiRuntimeServicesData is not appropriate
>> >>
>> >> Please use EfiReservedMemory instead.
>> >
>> > Specification (UEFI Spec 2_7_A Sept 6.pdf) describes
>> > EfiReservedMemoryType as  Not usable before and after ExitBootServices,
>> See Table 28 & 29 Hence EfiReservedMemoryType is not suitable in this
>> case.  Agree?
>> >
>>
>> It is not usable as ordinary memory, given that you turn it into 'special'
>> memory (with side effects) by turning it into a framebuffer.
>>
>> So EfiReservedMemory is perfectly appropriate here.
> [[Evan Lloyd]] First, I agree EfiReservedMemory is probably the sensible option.  The only alternative would be EfiMemoryMappedIO, and that, as you have pointed out in the past, introduces alignment requirements.  If you are happy to accept it, I'll ask Girish to go with that.

EfiReservedMemory is the only appropriate type to use here, and if the
spec is unclear, we should fix that.


>  However, Girish has a point, if only that the UEFI spec need more clarity on this, especially as https://lists.01.org/pipermail/edk2-devel/2017-February/007494.html pretty much confirms it is the right way to go.  In particular, the bald statement "EfiReservedMemoryType    Not usable.", seems unfortunate.
>
> Regards,
> Evan
>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel


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

end of thread, other threads:[~2018-03-22 17:32 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-20 16:18 [PATCH edk2-platforms v3 00/17] Update GOP Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 01/17] ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 02/17] ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding standard Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 03/17] ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 04/17] ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD inf Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 05/17] ARM/VExpressPkg: Add and update debug ASSERTS Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 06/17] ARM/VExpressPkg: PL111Lcd/HdLcd plaform libs: Minor code cleanup Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 07/17] ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32 Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 08/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check EFI_TIMEOUT Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 09/17] ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 10/17] ARM/VExpressPkg: Redefine LcdPlatformGetTimings function Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 11/17] ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData Girish Pathak
2018-03-21  3:37   ` Ard Biesheuvel
2018-03-21 11:07     ` Girish Pathak
2018-03-21 18:26       ` Ard Biesheuvel
2018-03-22 15:20         ` Evan Lloyd
2018-03-22 17:38           ` Ard Biesheuvel
2018-03-20 16:18 ` [PATCH edk2-platforms v3 13/17] ARM/VExpressPkg: Reserving framebuffer at build Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 14/17] ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 15/17] ARM/VExpressPkg: New DP500/DP550/DP650 platform library Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 16/17] ARM/JunoPkg: Adding SCMI MTL library Girish Pathak
2018-03-20 16:18 ` [PATCH edk2-platforms v3 17/17] ARM/JunoPkg: Add HDLCD platform library Girish Pathak
2018-03-21 12:56 ` [PATCH edk2-platforms v3 00/17] Update GOP Evan Lloyd

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