Re: [PATCH 18/19] ArmPlatformPkg: Reserving framebuffer at build

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 26 September 2017 at 21:15,  <evan.lloyd@arm.com> wrote:
> From: Girish Pathak <girish.pathak@arm.com>
>
> Currently frame buffer memory is either reserved in special VRAM or
> dynamically allocated using boot services memory allocation functions.
> When allocated using boot services calls the memory has to be allocated
> as EfiBootServicesData. Unfortunately failures have been seen with this
> case.  There is also an unfortunate lack of control on the placement of
> the frmae buffer.
>
> This change introduces two PCDs, PcdArmLcdFrameBufferBase and
> PcdArmLcdFrameBufferSize which enable build time reservation of the
> frame buffer, avoiding the need to allocate dynamically.  This allows
> the frame buffer to appear as "I/O memory" outside of the normal RAM
> map, which is similar to the "VRAM" case.
>
> This change has no impact on current code, only enables the option
> of build time reservation of frame buffers.
>

Where is the memory actually being reserved? And if it is reserved,
how can the OS reclaim it if it is not interested in using the GOP?


> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak <girish.pathak@arm.com>
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec                                           |  4 ++++
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf |  4 +++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c          | 21 ++++++++++++++++++--
>  3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 77eb789ad8fe4ddcbf25abefad2e7b7d3d5e1722..0174f63e77f5b8430e106289366feb9a6577fb99 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -111,6 +111,10 @@ [PcdsFixedAtBuild.common]
>    gArmPlatformTokenSpaceGuid.PcdPL111LcdBase|0x0|UINT32|0x00000026
>    gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase|0x0|UINT32|0x00000027
>
> +  ## If set, frame buffer memory will be reserved and mapped in the system RAM
> +  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize|0x0|UINT32|0x00000033
> +  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase|0x0|UINT64|0x00000034
> +
>    ## PL180 MCI
>    gArmPlatformTokenSpaceGuid.PcdPL180SysMciRegAddress|0x00000000|UINT32|0x00000028
>    gArmPlatformTokenSpaceGuid.PcdPL180MciBaseAddress|0x00000000|UINT32|0x00000029
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> index 9b16f7f0c4731ab72bfb1008a073e81842bae82b..60789e9b8ff1b936db04953a765fb164b0e85a40 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> @@ -1,5 +1,5 @@
>  #/* @file
> -#  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011-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
> @@ -57,6 +57,8 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdArmPrimaryCore
>
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
> +  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
> +  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
>
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> index 6379e81751fca5e7972c5c30f305be65fd1ae71d..5cd529750a3d2d3b0d381b58d875d378afaba2c2 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> @@ -1,6 +1,6 @@
>  /** @file
>  *
> -*  Copyright (c) 2011-2016, ARM Limited. All rights reserved.
> +*  Copyright (c) 2011-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
> @@ -20,8 +20,10 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <ArmPlatform.h>
>
> +#define FRAME_BUFFER_DESCRIPTOR ((FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0) ? 1 : 0)
> +
>  // Number of Virtual Memory Map Descriptors
> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          9
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + FRAME_BUFFER_DESCRIPTOR)
>
>  // DDR attributes
>  #define DDR_ATTRIBUTES_CACHED   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> @@ -142,6 +144,21 @@ ArmPlatformGetVirtualMemoryMap (
>    //
>    VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
>
> +  // Map region for the frame buffer in the system RAM
> +#if (FixedPcdGet32 (PcdArmLcdDdrFrameBufferSize) != 0)
> +  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;
> +#endif
> +
>    // Map sparse memory region if present
>    if (HasSparseMemory) {
>      VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase;
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>