[edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.

evan.lloyd@arm.com posted 18 patches 8 years, 1 month ago
[edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.
Posted by evan.lloyd@arm.com 8 years, 1 month ago
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>
---
 Platform/ARM/VExpressPkg/ArmVExpressPkg.dec                            |   3 +-
 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf         |  45 +++
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf |   3 +
 Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c           | 374 ++++++++++++++++++++
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c          |  10 +-
 5 files changed, 433 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec b/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
index 695553a94f7f7e963b5db995c5e54f1ae1559daf..a82bc905b3a2180e85f78a6c16aeba9fcb495bed 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-2017, 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.inf b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..4c8f3c3b3b6540505fce173ef16e2f3eafeb048b
--- /dev/null
+++ b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
@@ -0,0 +1,45 @@
+#/** @file ArmMaliDpLib.inf
+#
+#  Component description file for ArmMaliDpLib module
+#
+#  Copyright (c) 2017, 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
+  DxeServicesTableLib
+
+[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 c70c4ce64826174e6d15611de640ad3b902835b9..c2d8749d0c08d6afc71f7c41a27075d58ff27557 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
@@ -60,5 +60,8 @@ [FixedPcd]
   gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
   gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
 
+  gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase
+  gArmPlatformTokenSpaceGuid.PcdArmMaliDpMemoryRegionLength
+
 [Ppis]
   gArmMpCoreInfoPpiGuid
diff --git a/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..63f7c3b874b4fea80ab81c396a08e3e2e9b0581c
--- /dev/null
+++ b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
@@ -0,0 +1,374 @@
+/** @file ArmMaliDpLib.c
+
+  The file contains ARM Mali DP platform specific implementation.
+
+  Copyright (c) 2017, 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 <ArmPlatform.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/LcdPlatformLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.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 CONST 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 CONST 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                 CONST  VramSize,
+  OUT EFI_PHYSICAL_ADDRESS *CONST  VramBaseAddress
+  )
+{
+  EFI_STATUS Status;
+
+  // Check if memory is already reserved for the framebuffer.
+#if (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0)
+
+#if (!DP_VALID_BASE_ADDR (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase)))
+#error ARM Mali DP framebuffer base address cannot be wider than 40 bits.
+#else
+  *VramBaseAddress =
+    (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
+
+  Status = EFI_SUCCESS;
+#endif
+
+#else
+  // If not already reserved, attempt to allocate the VRAM from the DRAM.
+  Status = gBS->AllocatePages (
+                  AllocateAnyPages,
+                  EfiBootServicesData,
+                  EFI_SIZE_TO_PAGES (*VramSize),
+                  VramBaseAddress
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate framebuffer.\n"));
+    ASSERT (FALSE);
+    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;
+  }
+
+  // Mark the VRAM as write-combining. The VRAM is inside the DRAM, which is
+  // cacheable, for ARM/AArch64 EFI_MEMORY_WC memory is actually uncached.
+  Status = gDS->SetMemorySpaceAttributes (
+                  *VramBaseAddress,
+                  *VramSize,
+                  EFI_MEMORY_WC
+                  );
+  if (EFI_ERROR (Status)) {
+    ASSERT (FALSE);
+    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
+  }
+#endif
+
+  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             Frame buffer memory allocation success.
+  @retval EFI_UNSUPPORTED         Allocated address wider than 40 bits
+  @retval !EFI_SUCCESS            Other errors.
+**/
+EFI_STATUS
+LcdPlatformGetVram (
+  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
+  OUT UINTN                * CONST 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 CONST 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.
+**/
+EFI_STATUS
+LcdPlatformQueryMode (
+  IN  CONST UINT32                                 ModeNumber,
+  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info
+  )
+{
+  ASSERT (ModeNumber < mMaxMode);
+  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 CONST SCAN_TIMINGS ** Horizontal,
+  OUT CONST 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 value Bytes Per Pixel.
+
+  @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  CONST UINT32    ModeNumber,
+  OUT LCD_BPP * CONST Bpp
+  )
+{
+  ASSERT (Bpp != NULL);
+  if (ModeNumber >= mMaxMode) {
+    // Check valid ModeNumber.
+    ASSERT (ModeNumber < mMaxMode);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Bpp = LCD_BITS_PER_PIXEL_24;
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
index 1e8f3205312ebf30fa1666130bc944261384359a..f0b482f91a84d91ac9914c18b7941dd32ab8c8a7 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
@@ -21,9 +21,10 @@
 #include <ArmPlatform.h>
 
 #define FRAME_BUFFER_DESCRIPTOR ((FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0) ? 1 : 0)
+#define DP_BASE_DESCRIPTOR      ((FixedPcdGet64 (PcdArmMaliDpBase) != 0) ? 1 : 0)
 
 // Number of Virtual Memory Map Descriptors
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + FRAME_BUFFER_DESCRIPTOR)
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + FRAME_BUFFER_DESCRIPTOR + DP_BASE_DESCRIPTOR)
 
 // DDR attributes
 #define DDR_ATTRIBUTES_CACHED   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
@@ -158,6 +159,13 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
 #endif
 
+#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;
+#endif
   // Map sparse memory region if present
   if (HasSparseMemory) {
     VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase;
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.
Posted by Ard Biesheuvel 8 years, 1 month ago
On 22 December 2017 at 19:08,  <evan.lloyd@arm.com> wrote:
> 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>
> ---
>  Platform/ARM/VExpressPkg/ArmVExpressPkg.dec                            |   3 +-
>  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf         |  45 +++
>  Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf |   3 +
>  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c           | 374 ++++++++++++++++++++
>  Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c          |  10 +-
>  5 files changed, 433 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec b/Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
> index 695553a94f7f7e963b5db995c5e54f1ae1559daf..a82bc905b3a2180e85f78a6c16aeba9fcb495bed 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-2017, 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.inf b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..4c8f3c3b3b6540505fce173ef16e2f3eafeb048b
> --- /dev/null
> +++ b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> @@ -0,0 +1,45 @@
> +#/** @file ArmMaliDpLib.inf
> +#
> +#  Component description file for ArmMaliDpLib module
> +#
> +#  Copyright (c) 2017, 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
> +  DxeServicesTableLib
> +
> +[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 c70c4ce64826174e6d15611de640ad3b902835b9..c2d8749d0c08d6afc71f7c41a27075d58ff27557 100644
> --- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> +++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> @@ -60,5 +60,8 @@ [FixedPcd]
>    gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
>    gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
>
> +  gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase
> +  gArmPlatformTokenSpaceGuid.PcdArmMaliDpMemoryRegionLength
> +
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
> diff --git a/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..63f7c3b874b4fea80ab81c396a08e3e2e9b0581c
> --- /dev/null
> +++ b/Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
> @@ -0,0 +1,374 @@
> +/** @file ArmMaliDpLib.c
> +
> +  The file contains ARM Mali DP platform specific implementation.
> +
> +  Copyright (c) 2017, 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 <ArmPlatform.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/LcdPlatformLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.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 CONST 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 CONST 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);

Please fix the funky indentation

> +   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                 CONST  VramSize,
> +  OUT EFI_PHYSICAL_ADDRESS *CONST  VramBaseAddress
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  // Check if memory is already reserved for the framebuffer.
> +#if (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0)
> +

Please don't use CPP conditionals for control flow

> +#if (!DP_VALID_BASE_ADDR (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase)))
> +#error ARM Mali DP framebuffer base address cannot be wider than 40 bits.
> +#else
> +  *VramBaseAddress =
> +    (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
> +
> +  Status = EFI_SUCCESS;
> +#endif
> +
> +#else
> +  // If not already reserved, attempt to allocate the VRAM from the DRAM.
> +  Status = gBS->AllocatePages (
> +                  AllocateAnyPages,
> +                  EfiBootServicesData,
> +                  EFI_SIZE_TO_PAGES (*VramSize),
> +                  VramBaseAddress
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate framebuffer.\n"));
> +    ASSERT (FALSE);
> +    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;
> +  }
> +
> +  // Mark the VRAM as write-combining. The VRAM is inside the DRAM, which is
> +  // cacheable, for ARM/AArch64 EFI_MEMORY_WC memory is actually uncached.
> +  Status = gDS->SetMemorySpaceAttributes (
> +                  *VramBaseAddress,
> +                  *VramSize,
> +                  EFI_MEMORY_WC

Please add EFI_MEMORY_XP here

> +                  );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (FALSE);
> +    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
> +  }
> +#endif
> +
> +  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             Frame buffer memory allocation success.
> +  @retval EFI_UNSUPPORTED         Allocated address wider than 40 bits
> +  @retval !EFI_SUCCESS            Other errors.
> +**/
> +EFI_STATUS
> +LcdPlatformGetVram (
> +  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
> +  OUT UINTN                * CONST 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 CONST 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.
> +**/
> +EFI_STATUS
> +LcdPlatformQueryMode (
> +  IN  CONST UINT32                                 ModeNumber,
> +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info
> +  )
> +{
> +  ASSERT (ModeNumber < mMaxMode);
> +  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 CONST SCAN_TIMINGS ** Horizontal,
> +  OUT CONST 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 value Bytes Per Pixel.
> +
> +  @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  CONST UINT32    ModeNumber,
> +  OUT LCD_BPP * CONST Bpp
> +  )
> +{
> +  ASSERT (Bpp != NULL);
> +  if (ModeNumber >= mMaxMode) {
> +    // Check valid ModeNumber.
> +    ASSERT (ModeNumber < mMaxMode);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *Bpp = LCD_BITS_PER_PIXEL_24;
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> index 1e8f3205312ebf30fa1666130bc944261384359a..f0b482f91a84d91ac9914c18b7941dd32ab8c8a7 100644
> --- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> +++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> @@ -21,9 +21,10 @@
>  #include <ArmPlatform.h>
>
>  #define FRAME_BUFFER_DESCRIPTOR ((FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0) ? 1 : 0)
> +#define DP_BASE_DESCRIPTOR      ((FixedPcdGet64 (PcdArmMaliDpBase) != 0) ? 1 : 0)
>
>  // Number of Virtual Memory Map Descriptors
> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + FRAME_BUFFER_DESCRIPTOR)
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + FRAME_BUFFER_DESCRIPTOR + DP_BASE_DESCRIPTOR)
>
>  // DDR attributes
>  #define DDR_ATTRIBUTES_CACHED   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> @@ -158,6 +159,13 @@ ArmPlatformGetVirtualMemoryMap (
>    VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
>  #endif
>
> +#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;
> +#endif
>    // Map sparse memory region if present
>    if (HasSparseMemory) {
>      VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase;
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.
Posted by Evan Lloyd 8 years, 1 month ago

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 23 December 2017 16:07
> To: Evan Lloyd <Evan.Lloyd@arm.com>
> Cc: edk2-devel@lists.01.org; Arvind Chauhan <Arvind.Chauhan@arm.com>;
> Daniil Egranov <Daniil.Egranov@arm.com>; Thomas Abraham
> <thomas.abraham@arm.com>; "ard.biesheuvel@linaro.org"@arm.com;
> "leif.lindholm@linaro.org"@arm.com;
> "Matteo.Carlini@arm.com"@arm.com; "nd@arm.com"@arm.com
> Subject: Re: [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New
> DP500/DP550/DP650 platform library.
>
> On 22 December 2017 at 19:08,  <evan.lloyd@arm.com> wrote:
> > From: Girish Pathak <girish.pathak at arm.com>
> >
...
> > +
> > +/* 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                 CONST  VramSize,
> > +  OUT EFI_PHYSICAL_ADDRESS *CONST  VramBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS Status;
> > +
> > +  // Check if memory is already reserved for the framebuffer.
> > +#if (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0)
> > +
>
> Please don't use CPP conditionals for control flow
>
> > +#if (!DP_VALID_BASE_ADDR (FixedPcdGet64
> > +(PcdArmLcdDdrFrameBufferBase))) #error ARM Mali DP framebuffer
> base address cannot be wider than 40 bits.
> > +#else
> > +  *VramBaseAddress =
> > +    (EFI_PHYSICAL_ADDRESS)FixedPcdGet64
> > +(PcdArmLcdDdrFrameBufferBase);
> > +
> > +  Status = EFI_SUCCESS;
> > +#endif
> > +
> > +#else
> > +  // If not already reserved, attempt to allocate the VRAM from the
> DRAM.
> > +  Status = gBS->AllocatePages (
> > +                  AllocateAnyPages,
> > +                  EfiBootServicesData,
> > +                  EFI_SIZE_TO_PAGES (*VramSize),
> > +                  VramBaseAddress
> > +                  );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate
> framebuffer.\n"));
> > +    ASSERT (FALSE);
> > +    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;
> > +  }
> > +
> > +  // Mark the VRAM as write-combining. The VRAM is inside the DRAM,
> > + which is  // cacheable, for ARM/AArch64 EFI_MEMORY_WC memory is
> actually uncached.
> > +  Status = gDS->SetMemorySpaceAttributes (
> > +                  *VramBaseAddress,
> > +                  *VramSize,
> > +                  EFI_MEMORY_WC
>
> Please add EFI_MEMORY_XP here
>

 [[Evan Lloyd]] We can do that, happily.  However, in looking at this we found that the UEFI spec has in "2.3.6 AArch64 Platforms", section "2.3.6.1 Memory types":
EFI_MEMORY_XP, ...                                                                             Not used or defined

Does that suggest we need a minor spec update?

> > +                  );
> > +  if (EFI_ERROR (Status)) {
> > +    ASSERT (FALSE);
> > +    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES
> (*VramSize));
> > +  }
> > +#endif
> > +
> > +  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             Frame buffer memory allocation success.
> > +  @retval EFI_UNSUPPORTED         Allocated address wider than 40 bits
> > +  @retval !EFI_SUCCESS            Other errors.
> > +**/
> > +EFI_STATUS
> > +LcdPlatformGetVram (
> > +  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
> > +  OUT UINTN                * CONST 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 CONST 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.
> > +**/
> > +EFI_STATUS
> > +LcdPlatformQueryMode (
> > +  IN  CONST UINT32                                 ModeNumber,
> > +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info
> > +  )
> > +{
> > +  ASSERT (ModeNumber < mMaxMode);
> > +  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 CONST SCAN_TIMINGS ** Horizontal,
> > +  OUT CONST 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 value Bytes Per Pixel.
> > +
> > +  @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  CONST UINT32    ModeNumber,
> > +  OUT LCD_BPP * CONST Bpp
> > +  )
> > +{
> > +  ASSERT (Bpp != NULL);
> > +  if (ModeNumber >= mMaxMode) {
> > +    // Check valid ModeNumber.
> > +    ASSERT (ModeNumber < mMaxMode);
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  *Bpp = LCD_BITS_PER_PIXEL_24;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > diff --git
> >
> a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> >
> b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> > index
> >
> 1e8f3205312ebf30fa1666130bc944261384359a..f0b482f91a84d91ac9914
> c18b794
> > 1dd32ab8c8a7 100644
> > ---
> a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> > +++
> b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> > @@ -21,9 +21,10 @@
> >  #include <ArmPlatform.h>
> >
> >  #define FRAME_BUFFER_DESCRIPTOR ((FixedPcdGet64
> > (PcdArmLcdDdrFrameBufferBase) != 0) ? 1 : 0)
> > +#define DP_BASE_DESCRIPTOR      ((FixedPcdGet64
> (PcdArmMaliDpBase) != 0) ? 1 : 0)
> >
> >  // Number of Virtual Memory Map Descriptors -#define
> > MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 +
> FRAME_BUFFER_DESCRIPTOR)
> > +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 +
> > +FRAME_BUFFER_DESCRIPTOR + DP_BASE_DESCRIPTOR)
> >
> >  // DDR attributes
> >  #define DDR_ATTRIBUTES_CACHED
> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> > @@ -158,6 +159,13 @@ ArmPlatformGetVirtualMemoryMap (
> >    VirtualMemoryTable[Index].Attributes =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> >  #endif
> >
> > +#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;
> > +#endif
> >    // Map sparse memory region if present
> >    if (HasSparseMemory) {
> >      VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase;
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.
Posted by Alexei Fedorov 8 years ago
Hi Ard,


You wrote:


> > +  Status = gDS->SetMemorySpaceAttributes (
> > +                  *VramBaseAddress,
> > +                  *VramSize,
> > +                  EFI_MEMORY_WC
>
> Please add EFI_MEMORY_XP here
>


Setting EFI_MEMORY_XP causes assertion because  gDS->SetMemorySpaceAttributes() returns Unsupported in edk2\MdeModulePkg\Core\Dxe\Gcd\Gcd.c  with Status set at line #834:


      if ((Entry->Capabilities & Attributes) != Attributes) {

        //***AF
        DEBUG ((DEBUG_GCD, " @%d EFI_UNSUPPORTED: Capabilitie=0x%lX Attributes=0x%lX\n",
                __LINE__, Entry->Capabilities, Attributes));

        Status = EFI_UNSUPPORTED;
        goto Done;
      }

because Entry->Capabilities don't have EFI_MEMORY_XP set, see debug output below:


===================

 Framebuffer @0xF98A4000 0x4000000
GCD:SetMemorySpaceAttributes(Base=00000000F98A4000,Length=0000000004000000)
  Attributes  = 0000000000004002
 @834 EFI_UNSUPPORTED: Capabilities=0x800000000000000E Attributes=0x4002
  Status = Unsupported
GCDMemType Range                             Capabilities     Attributes
========== ================================= ================ ================
NonExist   0000000000000000-0000000007FFFFFF 0000000000000000 0000000000000000
MMIO       0000000008000000-000000000BFFFFFF C000000000000001 8000000000000001
NonExist   000000000C000000-000000001C16FFFF 0000000000000000 0000000000000000
MMIO       000000001C170000-000000001C170FFF C000000000000001 8000000000000001
NonExist   000000001C171000-000000004FFFFFFF 0000000000000000 0000000000000000
MMIO       0000000050000000-00000000501FFFFF C000000000000001 0000000000000001*
MMIO       0000000050200000-0000000057FFFFFF C000000000000001 0000000000000001
NonExist   0000000058000000-000000007FFFFFFF 0000000000000000 0000000000000000
SystemMem  0000000080000000-00000000FEFFFFFF 800000000000000E 0000000000000008*
NonExist   00000000FF000000-000000087FFFFFFF 0000000000000000 0000000000000000
SystemMem  0000000880000000-00000009FFFFFFFF 800000000000000E 0000000000000008*
NonExist   0000000A00000000-0000003FFFFFFFFF 0000000000000000 0000000000000000
MMIO       0000004000000000-00000040000FFFFF C000000000000001 0000000000000001*
MMIO       0000004000100000-00000040FFFFFFFF C000000000000001 0000000000000001
NonExist   0000004100000000-0000FFFFFFFFFFFF 0000000000000000 0000000000000000

ASSERT_EFI_ERROR (Status = Unsupported)
ASSERT [HdLcdGraphicsDxe] n:\edk2\ArmPlatformPkg\ArmJunoPkg\Library\HdLcdArmJunoLib\HdLcdArmJuno.c(254): !EFI_ERROR (Status)
===============

If  gDS->SetMemorySpaceAttributes() call is replaced with Cpu->SetMemoryAttributes(), attributes are set successfully with eXecute bit for frame buffer being cleared.


Alexei.

________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Evan Lloyd <Evan.Lloyd@arm.com>
Sent: 08 January 2018 18:51:12
To: Ard Biesheuvel
Cc: "Matteo.Carlini@arm.com"@arm.com; "leif.lindholm@linaro.org"@arm.com; "nd@arm.com"@arm.com; edk2-devel@lists.01.org; Arvind Chauhan; "ard.biesheuvel@linaro.org"@arm.com; Thomas Abraham
Subject: Re: [edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.



> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 23 December 2017 16:07
> To: Evan Lloyd <Evan.Lloyd@arm.com>
> Cc: edk2-devel@lists.01.org; Arvind Chauhan <Arvind.Chauhan@arm.com>;
> Daniil Egranov <Daniil.Egranov@arm.com>; Thomas Abraham
> <thomas.abraham@arm.com>; "ard.biesheuvel@linaro.org"@arm.com;
> "leif.lindholm@linaro.org"@arm.com;
> "Matteo.Carlini@arm.com"@arm.com; "nd@arm.com"@arm.com
> Subject: Re: [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New
> DP500/DP550/DP650 platform library.
>
> On 22 December 2017 at 19:08,  <evan.lloyd@arm.com> wrote:
> > From: Girish Pathak <girish.pathak at arm.com>
> >
...
> > +
> > +/* 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                 CONST  VramSize,
> > +  OUT EFI_PHYSICAL_ADDRESS *CONST  VramBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS Status;
> > +
> > +  // Check if memory is already reserved for the framebuffer.
> > +#if (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0)
> > +
>
> Please don't use CPP conditionals for control flow
>
> > +#if (!DP_VALID_BASE_ADDR (FixedPcdGet64
> > +(PcdArmLcdDdrFrameBufferBase))) #error ARM Mali DP framebuffer
> base address cannot be wider than 40 bits.
> > +#else
> > +  *VramBaseAddress =
> > +    (EFI_PHYSICAL_ADDRESS)FixedPcdGet64
> > +(PcdArmLcdDdrFrameBufferBase);
> > +
> > +  Status = EFI_SUCCESS;
> > +#endif
> > +
> > +#else
> > +  // If not already reserved, attempt to allocate the VRAM from the
> DRAM.
> > +  Status = gBS->AllocatePages (
> > +                  AllocateAnyPages,
> > +                  EfiBootServicesData,
> > +                  EFI_SIZE_TO_PAGES (*VramSize),
> > +                  VramBaseAddress
> > +                  );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate
> framebuffer.\n"));
> > +    ASSERT (FALSE);
> > +    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;
> > +  }
> > +
> > +  // Mark the VRAM as write-combining. The VRAM is inside the DRAM,
> > + which is  // cacheable, for ARM/AArch64 EFI_MEMORY_WC memory is
> actually uncached.
> > +  Status = gDS->SetMemorySpaceAttributes (
> > +                  *VramBaseAddress,
> > +                  *VramSize,
> > +                  EFI_MEMORY_WC
>
> Please add EFI_MEMORY_XP here
>

 [[Evan Lloyd]] We can do that, happily.  However, in looking at this we found that the UEFI spec has in "2.3.6 AArch64 Platforms", section "2.3.6.1 Memory types":
EFI_MEMORY_XP, ...                                                                             Not used or defined

Does that suggest we need a minor spec update?

> > +                  );
> > +  if (EFI_ERROR (Status)) {
> > +    ASSERT (FALSE);
> > +    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES
> (*VramSize));
> > +  }
> > +#endif
> > +
> > +  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             Frame buffer memory allocation success.
> > +  @retval EFI_UNSUPPORTED         Allocated address wider than 40 bits
> > +  @retval !EFI_SUCCESS            Other errors.
> > +**/
> > +EFI_STATUS
> > +LcdPlatformGetVram (
> > +  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
> > +  OUT UINTN                * CONST 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 CONST 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.
> > +**/
> > +EFI_STATUS
> > +LcdPlatformQueryMode (
> > +  IN  CONST UINT32                                 ModeNumber,
> > +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info
> > +  )
> > +{
> > +  ASSERT (ModeNumber < mMaxMode);
> > +  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 CONST SCAN_TIMINGS ** Horizontal,
> > +  OUT CONST 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 value Bytes Per Pixel.
> > +
> > +  @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  CONST UINT32    ModeNumber,
> > +  OUT LCD_BPP * CONST Bpp
> > +  )
> > +{
> > +  ASSERT (Bpp != NULL);
> > +  if (ModeNumber >= mMaxMode) {
> > +    // Check valid ModeNumber.
> > +    ASSERT (ModeNumber < mMaxMode);
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  *Bpp = LCD_BITS_PER_PIXEL_24;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > diff --git
> >
> a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> >
> b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> > index
> >
> 1e8f3205312ebf30fa1666130bc944261384359a..f0b482f91a84d91ac9914
> c18b794
> > 1dd32ab8c8a7 100644
> > ---
> a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> > +++
> b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> > @@ -21,9 +21,10 @@
> >  #include <ArmPlatform.h>
> >
> >  #define FRAME_BUFFER_DESCRIPTOR ((FixedPcdGet64
> > (PcdArmLcdDdrFrameBufferBase) != 0) ? 1 : 0)
> > +#define DP_BASE_DESCRIPTOR      ((FixedPcdGet64
> (PcdArmMaliDpBase) != 0) ? 1 : 0)
> >
> >  // Number of Virtual Memory Map Descriptors -#define
> > MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 +
> FRAME_BUFFER_DESCRIPTOR)
> > +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 +
> > +FRAME_BUFFER_DESCRIPTOR + DP_BASE_DESCRIPTOR)
> >
> >  // DDR attributes
> >  #define DDR_ATTRIBUTES_CACHED
> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> > @@ -158,6 +159,13 @@ ArmPlatformGetVirtualMemoryMap (
> >    VirtualMemoryTable[Index].Attributes =
> > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> >  #endif
> >
> > +#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;
> > +#endif
> >    // Map sparse memory region if present
> >    if (HasSparseMemory) {
> >      VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase;
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.
Posted by Ard Biesheuvel 8 years ago
On 24 January 2018 at 11:27, Alexei Fedorov <Alexei.Fedorov@arm.com> wrote:
> Hi Ard,
>
>
> You wrote:
>
>
>> > +  Status = gDS->SetMemorySpaceAttributes (
>> > +                  *VramBaseAddress,
>> > +                  *VramSize,
>> > +                  EFI_MEMORY_WC
>>
>> Please add EFI_MEMORY_XP here
>>
>
>
> Setting EFI_MEMORY_XP causes assertion because
> gDS->SetMemorySpaceAttributes() returns Unsupported in
> edk2\MdeModulePkg\Core\Dxe\Gcd\Gcd.c  with Status set at line #834:
>
>
>       if ((Entry->Capabilities & Attributes) != Attributes) {
>
>         //***AF
>         DEBUG ((DEBUG_GCD, " @%d EFI_UNSUPPORTED: Capabilitie=0x%lX
> Attributes=0x%lX\n",
>                 __LINE__, Entry->Capabilities, Attributes));
>
>         Status = EFI_UNSUPPORTED;
>         goto Done;
>       }
>
> because Entry->Capabilities don't have EFI_MEMORY_XP set, see debug output
> below:
>

<snip>

> If  gDS->SetMemorySpaceAttributes() call is replaced with
> Cpu->SetMemoryAttributes(), attributes are set successfully with eXecute bit
> for frame buffer being cleared.
>

Ugh. The GCD map conflates capabilities of the region (i.e., this
region can be configured as read-only by the MMU) with permissions
(i.e., the contents of this region should not be modified by the
program), which is why nobody bothers to set these attributes for the
GCD region. If you do set those bits in the GCD map, all allocations
carved out of it will be read-only and non-executable (because, hey,
why would you allocate writable or executable memory from a region
that can be configured as read-only or non-executable?).

In any case, your solution is the correct one: use the arch CPU
protocol directly instead (although there is something  in the PI spec
that bans it). Having an executable frame buffer is a much bigger deal
IMHO, especially when all other allocations are being locked down, and
so my preference is to switch to the arch CPU protocol here.

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