1. Patchew
  2. EDK2
  3. [edk2] [platforms: PATCH v2 0/8] Armada 7k/8k - memory improvements
  4. View patch

[edk2] [platforms: PATCH v2 6/8] Marvell/Armada: Enable dynamic DRAM size detection

Marcin Wojtas posted 8 patches 8 years, 1 month ago
There is a newer version of this series
[edk2] [platforms: PATCH v2 6/8] Marvell/Armada: Enable dynamic DRAM size detection
Posted by Marcin Wojtas 8 years, 1 month ago 
Instead of using hardcoded value in PcdSystemMemorySize PCD,
obtain DRAM size directly from SoC registers, which are filled
by firmware during early initialization stage.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c | 62 +++++++++++++++++++-
 Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h | 25 ++++++++
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
index 978e4d3..bf0ebcf 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
@@ -50,6 +50,60 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 STATIC ARM_MEMORY_REGION_DESCRIPTOR mVirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
 
+// Obtain DRAM size basing on register values filled by early firmware.
+STATIC
+UINT64
+GetDramSize (
+  IN OUT UINT64 *MemSize
+  )
+{
+  UINT64 BaseAddr;
+  UINT8 RegionCode;
+  UINT8 Cs;
+
+  *MemSize = 0;
+
+  for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) {
+
+    /* Exit loop on first disabled DRAM CS */
+    if (!DRAM_CS_ENABLED (Cs)) {
+      break;
+    }
+
+    /*
+     * Sanity check for base address of next DRAM block.
+     * Only continuous space will be used.
+     */
+    BaseAddr = GET_DRAM_REGION_BASE (Cs);
+    if (BaseAddr != *MemSize) {
+      DEBUG ((DEBUG_ERROR,
+        "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n",
+        __FUNCTION__,
+        *MemSize));
+      return EFI_SUCCESS;
+    }
+
+    /* Decode area length for current CS from register value */
+    RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs);
+    if (DRAM_REGION_SIZE_CODE_INVALID (RegionCode)) {
+      DEBUG ((DEBUG_ERROR,
+        "%a: Invalid memory region code (0x%x) for CS#%d\n",
+        __FUNCTION__,
+        RegionCode,
+        Cs));
+      return EFI_INVALID_PARAMETER;
+    }
+
+    if (RegionCode <= 0x4) {
+      *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode);
+    } else {
+      *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode);
+    }
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Return the Virtual Memory Map of your platform
 
@@ -72,12 +126,18 @@ ArmPlatformGetVirtualMemoryMap (
   UINT64                        MemHighSize;
   UINT64                        ConfigSpaceBaseAddr;
   EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
+  EFI_STATUS                    Status;
 
   ASSERT (VirtualMemoryMap != NULL);
 
   ConfigSpaceBaseAddr = FixedPcdGet64 (PcdConfigSpaceBaseAddress);
 
-  MemSize = FixedPcdGet64 (PcdSystemMemorySize);
+  // Obtain total memory size from the hardware.
+  Status = GetDramSize (&MemSize);
+  if (EFI_ERROR (Status)) {
+    MemSize = FixedPcdGet64 (PcdSystemMemorySize);
+    DEBUG ((DEBUG_ERROR, "Limit total memory size to %d MB\n", MemSize / 1024 / 1024));
+  }
 
   if (DRAM_REMAP_ENABLED) {
     MemLowSize = MIN (DRAM_REMAP_TARGET, MemSize);
diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
index 8101cf3..dd218d9 100644
--- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
+++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
@@ -46,3 +46,28 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
           (MmioRead32 (CCU_MC_RCR_REG) & REMAP_SIZE_MASK) + SIZE_1MB
 #define DRAM_REMAP_TARGET \
           (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS)
+
+#define DRAM_CH0_MMAP_LOW_REG(cs)       (0xf0020200 + (cs) * 0x8)
+#define DRAM_CS_VALID_ENABLED_MASK      0x1
+#define DRAM_AREA_LENGTH_OFFS           16
+#define DRAM_AREA_LENGTH_MASK           (0x1f << DRAM_AREA_LENGTH_OFFS)
+#define DRAM_START_ADDRESS_L_OFFS       23
+#define DRAM_START_ADDRESS_L_MASK       (0x1ff << DRAM_START_ADDRESS_L_OFFS)
+#define DRAM_CH0_MMAP_HIGH_REG(cs)      (0xf0020204 + (cs) * 0x8)
+#define DRAM_START_ADDR_HTOL_OFFS       32
+
+#define DRAM_MAX_CS_NUM                 8
+
+#define DRAM_CS_ENABLED(Cs) \
+          (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENABLED_MASK)
+#define GET_DRAM_REGION_BASE(Cs) \
+          ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \
+           DRAM_START_ADDR_HTOL_OFFS) | \
+          (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRESS_L_MASK);
+#define GET_DRAM_REGION_SIZE_CODE(Cs) \
+          (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \
+           DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS
+#define DRAM_REGION_SIZE_CODE_INVALID(C) \
+          (((C) > 0x4 && (C) < 0x7) || (C) > 0x1b)
+#define GET_DRAM_REGION_SIZE_ODD(C)     ((UINT64)0x18000000 << (C))
+#define GET_DRAM_REGION_SIZE_EVEN(C)    ((UINT64)1 << ((C) + 16))
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 6/8] Marvell/Armada: Enable dynamic DRAM size detection
Posted by Leif Lindholm 8 years, 1 month ago 
On Wed, Oct 25, 2017 at 08:45:28AM +0200, Marcin Wojtas wrote:
> Instead of using hardcoded value in PcdSystemMemorySize PCD,
> obtain DRAM size directly from SoC registers, which are filled
> by firmware during early initialization stage.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c | 62 +++++++++++++++++++-
>  Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h | 25 ++++++++
>  2 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> index 978e4d3..bf0ebcf 100644
> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
> @@ -50,6 +50,60 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  
>  STATIC ARM_MEMORY_REGION_DESCRIPTOR mVirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
>  
> +// Obtain DRAM size basing on register values filled by early firmware.
> +STATIC
> +UINT64
> +GetDramSize (
> +  IN OUT UINT64 *MemSize
> +  )
> +{
> +  UINT64 BaseAddr;
> +  UINT8 RegionCode;
> +  UINT8 Cs;
> +
> +  *MemSize = 0;
> +
> +  for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) {
> +
> +    /* Exit loop on first disabled DRAM CS */
> +    if (!DRAM_CS_ENABLED (Cs)) {
> +      break;
> +    }
> +
> +    /*
> +     * Sanity check for base address of next DRAM block.
> +     * Only continuous space will be used.
> +     */
> +    BaseAddr = GET_DRAM_REGION_BASE (Cs);
> +    if (BaseAddr != *MemSize) {
> +      DEBUG ((DEBUG_ERROR,
> +        "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n",
> +        __FUNCTION__,
> +        *MemSize));
> +      return EFI_SUCCESS;
> +    }
> +
> +    /* Decode area length for current CS from register value */
> +    RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs);
> +    if (DRAM_REGION_SIZE_CODE_INVALID (RegionCode)) {
> +      DEBUG ((DEBUG_ERROR,
> +        "%a: Invalid memory region code (0x%x) for CS#%d\n",
> +        __FUNCTION__,
> +        RegionCode,
> +        Cs));
> +      return EFI_INVALID_PARAMETER;
> +    }
> +
> +    if (RegionCode <= 0x4) {

It would be even nicer this one could be abstracted away in another
macro (now everything else is this clean, it does sort of stand out).

> +      *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode);
> +    } else {
> +      *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode);
> +    }

I guess we should also check for the reserved ones.
I'm also a fan of the "common case first" approach to if-statements.

So, could we squash these two together?:
if (DRAM_REGION_SIZE_EVEN (RegionCode)) {
  *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode);
} else if (DRAM_REGION_SIZE_ODD (RegionCode)) {
  *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode);
} else {
  DEBUG ((DEBUG_ERROR,
    "%a: Invalid memory region code (0x%x) for CS#%d\n",
    __FUNCTION__,
    RegionCode,
    Cs));
    return EFI_INVALID_PARAMETER;
}

Where
#define DRAM_REGION_SIZE_ODD(x)  ((x) <= 4)
#define DRAM_REGION_SIZE_EVEN(x) (((x) >= 7) && ((x) <= 26))

(I use decimal numbers, because so does the documentation.)

/
    Leif

> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Return the Virtual Memory Map of your platform
>  
> @@ -72,12 +126,18 @@ ArmPlatformGetVirtualMemoryMap (
>    UINT64                        MemHighSize;
>    UINT64                        ConfigSpaceBaseAddr;
>    EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
> +  EFI_STATUS                    Status;
>  
>    ASSERT (VirtualMemoryMap != NULL);
>  
>    ConfigSpaceBaseAddr = FixedPcdGet64 (PcdConfigSpaceBaseAddress);
>  
> -  MemSize = FixedPcdGet64 (PcdSystemMemorySize);
> +  // Obtain total memory size from the hardware.
> +  Status = GetDramSize (&MemSize);
> +  if (EFI_ERROR (Status)) {
> +    MemSize = FixedPcdGet64 (PcdSystemMemorySize);
> +    DEBUG ((DEBUG_ERROR, "Limit total memory size to %d MB\n", MemSize / 1024 / 1024));
> +  }
>  
>    if (DRAM_REMAP_ENABLED) {
>      MemLowSize = MIN (DRAM_REMAP_TARGET, MemSize);
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
> index 8101cf3..dd218d9 100644
> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.h
> @@ -46,3 +46,28 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>            (MmioRead32 (CCU_MC_RCR_REG) & REMAP_SIZE_MASK) + SIZE_1MB
>  #define DRAM_REMAP_TARGET \
>            (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS)
> +
> +#define DRAM_CH0_MMAP_LOW_REG(cs)       (0xf0020200 + (cs) * 0x8)
> +#define DRAM_CS_VALID_ENABLED_MASK      0x1
> +#define DRAM_AREA_LENGTH_OFFS           16
> +#define DRAM_AREA_LENGTH_MASK           (0x1f << DRAM_AREA_LENGTH_OFFS)
> +#define DRAM_START_ADDRESS_L_OFFS       23
> +#define DRAM_START_ADDRESS_L_MASK       (0x1ff << DRAM_START_ADDRESS_L_OFFS)
> +#define DRAM_CH0_MMAP_HIGH_REG(cs)      (0xf0020204 + (cs) * 0x8)
> +#define DRAM_START_ADDR_HTOL_OFFS       32
> +
> +#define DRAM_MAX_CS_NUM                 8
> +
> +#define DRAM_CS_ENABLED(Cs) \
> +          (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENABLED_MASK)
> +#define GET_DRAM_REGION_BASE(Cs) \
> +          ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \
> +           DRAM_START_ADDR_HTOL_OFFS) | \
> +          (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRESS_L_MASK);
> +#define GET_DRAM_REGION_SIZE_CODE(Cs) \
> +          (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \
> +           DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS
> +#define DRAM_REGION_SIZE_CODE_INVALID(C) \
> +          (((C) > 0x4 && (C) < 0x7) || (C) > 0x1b)
> +#define GET_DRAM_REGION_SIZE_ODD(C)     ((UINT64)0x18000000 << (C))
> +#define GET_DRAM_REGION_SIZE_EVEN(C)    ((UINT64)1 << ((C) + 16))
> -- 
> 2.7.4
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.