On 22 December 2017 at 19:08, <evan.ll...@arm.com> wrote:
> 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.pat...@arm.com>
> Signed-off-by: Evan Lloyd <evan.ll...@arm.com>
> ---
> Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf | 4
> +++-
> Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c | 20
> ++++++++++++++++++--
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git
> a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> index
> 4cbd2ff4b4faf11ccd4fe30117708d7cc4c1bf0e..c70c4ce64826174e6d15611de640ad3b902835b9
> 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-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
>
As mentioned in the review of the other patch, please move these into
VExpressPkg
> [Ppis]
> gArmMpCoreInfoPpiGuid
> diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> index
> 6379e81751fca5e7972c5c30f305be65fd1ae71d..1e8f3205312ebf30fa1666130bc944261384359a
> 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-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,20 @@ 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
> +
Whose responsibility is it to ensure that this region is removed from
SystemMemoryBase/SystemMemorySize? If it is up to the .DSC author,
could you please add an ASSERT() here that they don't overlap?
> // 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
