From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::241; helo=mail-it0-x241.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x241.google.com (mail-it0-x241.google.com [IPv6:2607:f8b0:4001:c0b::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 C75EE222F4E0F for ; Sat, 23 Dec 2017 07:58:08 -0800 (PST) Received: by mail-it0-x241.google.com with SMTP id m11so20199843iti.1 for ; Sat, 23 Dec 2017 08:02:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Libnin6vBeDh4OgaEIjTLc+O8oMQfnwQIhVN18x86ik=; b=BoKDp2eeQ1Hn9/jgG8t/k2+z0bTGteSGX4YXz9i8RoY/rjIL0553Uw8h+zToRJSQWe er74q4E+PpQG2/Zr9E7KybG+DyeGgNsZy3Mt/q+Fu0IZoeQQhYgSkpUY3wN6xjofYmWX /AKrnfxeV+183RvJcGd1tgJSgl0aXHztH9XO0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Libnin6vBeDh4OgaEIjTLc+O8oMQfnwQIhVN18x86ik=; b=hNiUWkY9Ui7Lana72TU+RA/0LzudcnjkCsypaiWEMkG1PkJqLzTdnbjP9egZ+tZubX pyULvTf5xQyDXzGvn4/1yfyjVnpkNu90Rbo/L2lzbY0hmsXM7hNQXWLqJhBVsYl7EBEO FftrMbZo4WlKp7GB+k3oKywrrxcX2SJshVsGQYRCqtJ0rJoyil9lZzkofPDxZQR2SXP7 7Dxbd4xWhAG5nB5SzCyEurEEoj7pxTjKbqw87XMHcP5Vu5bTjT9erKt+qeWo2PM262tg Sza3JTXxh1J/OeKW/D1+GLzPR1+glJwjGM3KhYA9VyR+vnm/79uDrf6KC5Fqy2Quw6fi mSJg== X-Gm-Message-State: AKGB3mIQuE4sXHjw4iEmretAtW3UqOGJ7DJZGzwAMvhct6AhaIypKCV5 +xh/yvMnbHTjEs5Ha95ixeNlK2H5L2eeAqKWbPiE6g== X-Google-Smtp-Source: ACJfBotbwQBveIXMElDasXMn/AwO9ZY367xLldAnGafimxu+oAuk4DyWE+2xHQU0DtX1K7uI3y5bmNgc8a8ec8nBD5k= X-Received: by 10.36.55.138 with SMTP id r132mr22479728itr.34.1514044979223; Sat, 23 Dec 2017 08:02:59 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.52.14 with HTTP; Sat, 23 Dec 2017 08:02:58 -0800 (PST) In-Reply-To: <20171222190821.12440-15-evan.lloyd@arm.com> References: <20171222190821.12440-1-evan.lloyd@arm.com> <20171222190821.12440-15-evan.lloyd@arm.com> From: Ard Biesheuvel Date: Sat, 23 Dec 2017 16:02:58 +0000 Message-ID: To: Evan Lloyd Cc: "edk2-devel@lists.01.org" , Arvind Chauhan , Daniil Egranov , Thomas Panakamattam Abraham , <"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 14/18] ARM/VExpressPkg: Reserving framebuffer at build X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Dec 2017 15:58:09 -0000 Content-Type: text/plain; charset="UTF-8" On 22 December 2017 at 19:08, wrote: > From: Girish Pathak > > 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 > Signed-off-by: Evan Lloyd > --- > 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 > #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,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") >