From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::241; helo=mail-wr0-x241.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x241.google.com (mail-wr0-x241.google.com [IPv6:2a00:1450:400c:c0c::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 11FF7202E617B for ; Wed, 25 Oct 2017 07:47:26 -0700 (PDT) Received: by mail-wr0-x241.google.com with SMTP id w105so279158wrc.0 for ; Wed, 25 Oct 2017 07:51:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=3od0vBMeHjjxcNTKNuE39eJ8UpvKCZ4lI8ub0FnI1U4=; b=FBXGilDRLJzdKQ+JxwwTc2vnZkC2dF9hyQGwfjyXJf+oE0yckdMAW2YV1VIGNJ3Ovn pUTxuzAyMqKl3ueq870dMGNZtrtbjoOSCajJXq0sYFmKhDMaZz/h+5MfaIieqcHQ5m1s C8t+io6RAvDONhlF/xL+RHi9euG+oO9ocXUj4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=3od0vBMeHjjxcNTKNuE39eJ8UpvKCZ4lI8ub0FnI1U4=; b=G24QQxsY1IENiBuyTpB83lh9Mu4rd5QaIW49A0WQpsmLojwUuQBUDuFyrk71D89txs dBg0Pj8elVuoTmbc+0Vy50R09dB2uTP5QJx+8RIUlC553g/movZJRLzLbhixDrvWK7yw T0lTmOosP00Bnayw0wzYgjxLXBlkYtnLOb5Olph6Vw/IKbumV0BEHT+QqSCXUPc9GLit a1XKKmOPcfFs7VXM7P+JpUEYOkFxWlSD8IFw0+6HwII6P7V1pHe/aW6dUs0DDWZjhvtw 21h6AlhScSFm0tR28bBUgX3xcqIccqrDXWgP76rjWoy+wtABx71K6vTZjVLe9LAyfTRQ xePw== X-Gm-Message-State: AMCzsaV6t+3z7tK42SAROqi6ErtJ3iAyLzGgwqJPkyUrp9CrcclnMxKv Yj2BJBqliXgCARLzX/s/ZDVDGw== X-Google-Smtp-Source: ABhQp+SyRuUH80IjQC8ob/4FPQ+ADB+JTwS9NYgXmyyCGJFvSz4yUKMoFrdicTTFpI0VQdJOHYJ6AQ== X-Received: by 10.223.144.76 with SMTP id h70mr2636929wrh.228.1508943070487; Wed, 25 Oct 2017 07:51:10 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id f27sm3956036wrf.63.2017.10.25.07.51.09 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 25 Oct 2017 07:51:09 -0700 (PDT) Date: Wed, 25 Oct 2017 15:51:07 +0100 From: Leif Lindholm To: evan.lloyd@arm.com Cc: edk2-devel@lists.01.org Message-ID: <20171025145107.54wcxulvdano7dxx@bivouac.eciton.net> References: <20170926201529.11644-1-evan.lloyd@arm.com> <20170926201529.11644-19-evan.lloyd@arm.com> MIME-Version: 1.0 In-Reply-To: <20170926201529.11644-19-evan.lloyd@arm.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH 18/19] ArmPlatformPkg: Reserving framebuffer at build X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Oct 2017 14:47:27 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Sep 26, 2017 at 09:15:28PM +0100, evan.lloyd@arm.com wrote: > From: Girish Pathak > > 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Girish Pathak > Signed-off-by: Evan Lloyd > --- > 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 > #include > > +#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. Well done on that note, I was about to object, because I keep forgetting about that twist :) Reviewed-by: Leif Lindholm > + */ > + 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") >