Cache the SDRAM once it is verified (cache=on)

configs/README.md

@@ -76,6 +76,7 @@ explicitly force 16-bit width for the same reason.
 | `timeout_ms`       | integer                         | `3000`  | Upper bound on the whole configuration sequence. |
 | `heap_usage`       | `none`, `heap`                   | `none`  | `heap` registers the mapped SDRAM as an additional dmheap context. |
 | `heap_alignment`   | integer (bytes)                 | `sizeof(void*)` | Alignment used when registering the dmheap context. |
+| `cache`            | `on` / `off`                    | `on`          | Let the CPU cache the SDRAM (write-back) once it is verified - checked through the cache first, left uncached when that fails. `off` keeps it uncached. |
 | `interrupt_handler`| dmhaman handler name             | -       | Optional: called on SDRAM refresh-error interrupts. |
 
 ## Board Configurations

docs/configuration.md

@@ -14,6 +14,7 @@ data_bus_width=32       ; 0 (chip default), 8, 16 or 32
 timeout_ms=3000         ; upper bound on the whole configuration sequence
 heap_usage=heap         ; none | heap
 heap_alignment=4        ; only used when heap_usage=heap
+cache=on                ; on (default) | off - let the CPU cache the memory
 interrupt_handler=my_handler  ; optional dmhaman handler name for refresh-error events
 ```
 

docs/dmfmc.md

@@ -76,7 +76,16 @@ them in file order before configuring `dmfmc`, exactly like it does for
    the problem instead of working around it.
 6. `dmfmc_port_finish_sdram_initialization()` programs the refresh timer
    (`FMC_SDRTR`) from the chip's `auto_refresh_period_us`.
-7. If `heap_usage=heap`, the mapped region is handed to `dmheap_init()` as a
+7. With `cache=on` (the default) `dmfmc_port_cache_sdram()` makes the window
+   cacheable (MPU: Normal, write-back, read/write allocate) and checks it
+   through the data cache: 64 KiB of patterns written into the cache,
+   cleaned to the SDRAM, invalidated and read back from it, twice. When that
+   fails, the window is made uncached again, as it is during bring-up. On a
+   Cortex-M7 the caches are what makes code and data in the SDRAM fast - the
+   drivers that let other bus masters see cached memory clean and invalidate
+   it by address (dmsdio, dmlcdtft). Without a data cache (or with it off)
+   only the attributes are set.
+8. If `heap_usage=heap`, the mapped region is handed to `dmheap_init()` as a
    brand new heap context (dmheap supports any number of independent
    contexts - see its README for the same `kernel_ctx`/`extram_ctx` pattern).
 

include/dmfmc.h

@@ -16,6 +16,7 @@ typedef struct
     uint32_t                 configuration_timeout_ms;  /**< Maximum time allowed for the whole configuration sequence */
     dmfmc_heap_usage_t       heap_usage;                /**< Whether to register the mapped memory with dmheap */
     uint32_t                 heap_alignment;            /**< Alignment to use when registering the dmheap context */
+    bool                     cache;                     /**< Let the CPU cache the memory (cache=on, the default) */
     dmfmc_interrupt_handler_t interrupt_handler;        /**< Interrupt handler (NULL = not used) */
 } dmfmc_config_t;
 

include/dmfmc_port.h

@@ -34,6 +34,16 @@ dmod_dmfmc_port_api(1.0, int, _sdram_send_command,
 dmod_dmfmc_port_api(1.0, int, _finish_sdram_initialization,
     ( dmfmc_sdram_bank_t bank, const dmfmc_sdram_chip_params_t *chip ) );
 
+/**
+ * Lets the CPU cache the SDRAM window (write-back) once it is verified. The
+ * port checks the memory through the cache - patterns written into the
+ * cache, cleaned to the SDRAM and read back from it - and leaves the window
+ * uncached when that fails. A core without a data cache only gets the
+ * attributes. Returns 0, or -EIO when the check failed.
+ */
+dmod_dmfmc_port_api(1.0, int, _cache_sdram,
+    ( dmfmc_sdram_bank_t bank, void *start, uint32_t size ) );
+
 /* --- NOR / NAND / PSRAM configuration ---
  *
  * Declared for API completeness; the STM32 port currently returns -ENOSYS

include/dmfmc_types.h

@@ -2,9 +2,9 @@
 #define DMFMC_TYPES_H
 
 #include <stdint.h>
-#include "dmdrvi_ioctl.h"
 #include <stdbool.h>
 #include <stddef.h>
+#include "dmdrvi_ioctl.h"
 
 /**
  * @brief Type of memory attached to the FMC

src/dmfmc.c

@@ -161,6 +161,7 @@ static int read_config_parameters(dmdrvi_context_t context, dmini_context_t conf
     context->config.configuration_timeout_ms = (uint32_t)dmini_get_int(config, section, "timeout_ms", 3000);
     context->config.heap_usage     = string_to_heap_usage(dmini_get_string(config, section, "heap_usage", "none"));
     context->config.heap_alignment = (uint32_t)dmini_get_int(config, section, "heap_alignment", (int)sizeof(void *));
+    context->config.cache          = strcmp(dmini_get_string(config, section, "cache", "on"), "off") != 0;
     context->config.interrupt_handler = NULL;
 
     const char *chip_name = dmini_get_string(config, section, "chip", NULL);
@@ -281,6 +282,11 @@ static int configure(dmdrvi_context_t context)
         return ret;
     }
 
+    /* Verified uncached: now let the CPU cache it. The port checks the
+     * memory through the cache too and keeps it uncached if that fails. */
+    if (c->cache && dmfmc_port_cache_sdram(c->bank, context->result.memory_start, context->result.memory_size_bytes) != 0)
+        DMOD_LOG_WARN("FMC: the SDRAM stays uncached\n");
+
     if (context->interrupt_handler_name != NULL)
     {
         if (dmfmc_port_add_interrupt_handler(internal_interrupt_handler, context) != 0)

src/port/stm32_common/stm32_common.c

@@ -182,30 +182,41 @@ static uint32_t pick_cas_latency_cycles(const dmfmc_sdram_chip_params_t *chip, u
  * whatever a future user of the MPU picks first. */
 #define DMFMC_MPU_SDRAM_REGION  6U
 
+/* MPU attributes of the SDRAM window (RASR TEX/C/B) */
+#define MPU_RASR_ATTR_Msk       ((7U << 19) | (1U << 17) | (1U << 16))
+#define MPU_RASR_NONCACHEABLE   (1U << 19)                              /* TEX=001 C=0 B=0: Normal, non-cacheable */
+#define MPU_RASR_WRITE_BACK     ((1U << 19) | (1U << 17) | (1U << 16))  /* TEX=001 C=1 B=1: Normal, write-back, read/write allocate */
+
+/* Cortex-M7 data cache (SCB) - CCR.DC is 0 on a core without one */
+#define SCB_CCR                 (*(volatile uint32_t *)0xE000ED14UL)
+#define SCB_CCR_DC              (1UL << 16)
+#define SCB_DCIMVAC             (*(volatile uint32_t *)0xE000EF5CUL)   /* invalidate by address */
+#define SCB_DCCMVAC             (*(volatile uint32_t *)0xE000EF68UL)   /* clean by address */
+#define SCB_DCCIMVAC            (*(volatile uint32_t *)0xE000EF70UL)   /* clean and invalidate by address */
+#define DCACHE_LINE             32U
+
+/* Bytes of the SDRAM checked through the cache - many times the data cache,
+ * so the patterns are evicted to the SDRAM in bursts as well as cleaned */
+#define CACHE_CHECK_SIZE        (64U * 1024U)
+
 /**
- * @brief Mark the FMC SDRAM window as Normal, non-cacheable, non-bufferable
- * memory via the MPU.
+ * @brief Mark the FMC SDRAM window as Normal, non-cacheable memory via the
+ * MPU - until it is verified (see dmfmc_port_cache_sdram()).
  *
  * Without an explicit MPU region, this address range falls back to the
- * Cortex-M7 default background map, and on this core/board combination that
- * turned out to allow the store buffer to merge/reorder back-to-back writes
- * to nearby addresses in this window - only the last of several rapid
- * consecutive writes actually reached the SDRAM, exactly the "every word
- * reads back as the last one written" corruption this driver hit during
- * bring-up. B=0 (non-bufferable) forces every store to complete, in order,
- * before the next one begins, which is what actually fixed that corruption.
+ * Cortex-M7 default background map (0xC0000000: Device memory). During
+ * bring-up, writes then seemed to get lost - "every word reads back as the
+ * last one written"; the bank target bits of the commands were swapped
+ * (FMC_SDCMR CTB1/CTB2) and the SDRAM clock was too fast for RBURST at the
+ * time, and the region was made non-bufferable as well. With those fixed,
+ * the memory holds data through the cache, which dmfmc_port_cache_sdram()
+ * checks before it turns caching on.
  *
- * This region was originally configured as Strongly Ordered (TEX=000) for
- * the same B=0/C=0 effect, but ARMv7-M forbids *any* unaligned access to
- * Strongly-Ordered (or Device) memory - it hard-faults, always, regardless
- * of the core's normal (non-trapping) unaligned-access support for Normal
- * memory. That bit us for real once SDRAM was registered as a dmheap
- * default context: dmod's module loader parses loaded modules' internal
- * structures (headers/footers) with plain struct-pointer dereferences that
- * do not guarantee 4-byte alignment of every field, and a module placed in
- * this SDRAM region hit exactly that fault. TEX=001 (Normal, non-cacheable)
- * keeps the same non-bufferable/non-cacheable guarantees while permitting
- * unaligned accesses.
+ * TEX=001 (Normal) rather than Strongly Ordered: ARMv7-M forbids *any*
+ * unaligned access to Strongly-Ordered (or Device) memory - it hard-faults,
+ * always - and dmod's module loader parses loaded modules' internal
+ * structures with plain struct-pointer dereferences that do not guarantee
+ * 4-byte alignment of every field.
  *
  * Safe to call unconditionally: if the MPU is already enabled by someone
  * else, only a new region is added; if it is not yet enabled, it is enabled
@@ -221,10 +232,7 @@ static void configure_sdram_mpu_region(void)
     MPU_RBAR = STM32_FMC_SDRAM_BANK1_BASE; /* 16MB-aligned; region selected via RNR, so VALID/REGION bits stay 0 */
     MPU_RASR = (1U << 0)      /* ENABLE */
              | (23U << 1)     /* SIZE: 2^(23+1) = 16 MiB, covers this bank's chip capacity */
-             | (0U << 16)     /* B = 0: non-bufferable */
-             | (0U << 17)     /* C = 0: non-cacheable */
-             | (0U << 18)     /* S = 0 */
-             | (1U << 19)     /* TEX = 001 -> with C=B=0, Normal non-cacheable (permits unaligned access, unlike Strongly Ordered) */
+             | MPU_RASR_NONCACHEABLE
              | (0x3U << 24);  /* AP = 011: full read/write access, any privilege level.
                                 * XN deliberately left 0 (executable): when this SDRAM is
                                 * registered as a dmheap default context, dmod's own module
@@ -236,6 +244,53 @@ static void configure_sdram_mpu_region(void)
 
     if ((MPU_CTRL & 0x1U) == 0U)
         MPU_CTRL |= (1U << 0) | (1U << 2); /* ENABLE | PRIVDEFENA */
+    __asm volatile ("dsb" ::: "memory");
+    __asm volatile ("isb" ::: "memory");
+}
+
+static void set_sdram_attributes(uint32_t attributes)
+{
+    MPU_RNR  = DMFMC_MPU_SDRAM_REGION;
+    MPU_RASR = (MPU_RASR & ~MPU_RASR_ATTR_Msk) | attributes;
+    __asm volatile ("dsb" ::: "memory");
+    __asm volatile ("isb" ::: "memory");
+}
+
+/* `operation` (clean / invalidate by address) on every cache line of the range */
+static void dcache_lines(volatile uint32_t *operation, const void *start, uint32_t size)
+{
+    uintptr_t line = (uintptr_t)start & ~(uintptr_t)(DCACHE_LINE - 1U);
+    uintptr_t end  = (uintptr_t)start + size;
+    __asm volatile ("dsb" ::: "memory");
+    for (; line < end; line += DCACHE_LINE)
+        *operation = (uint32_t)line;
+    __asm volatile ("dsb" ::: "memory");
+}
+
+/* The pattern of word `i` in pass `pass` - different in every word and pass */
+static inline uint32_t check_pattern(uint32_t i, uint32_t pass)
+{
+    uint32_t v = (i + 1U) * 2654435761U;
+    return (pass == 0U) ? v : ~v;
+}
+
+/* Write the patterns through the cache, push them to the SDRAM, forget them
+ * and read them back from it: every word must hold its own pattern */
+static bool check_through_cache(void *start, uint32_t size)
+{
+    volatile uint32_t *words = (volatile uint32_t *)start;
+    uint32_t count = ((size < CACHE_CHECK_SIZE) ? size : CACHE_CHECK_SIZE) / sizeof(uint32_t);
+    bool ok = true;
+    for (uint32_t pass = 0; pass < 2U && ok; pass++)
+    {
+        for (uint32_t i = 0; i < count; i++)
+            words[i] = check_pattern(i, pass);
+        dcache_lines(&SCB_DCCIMVAC, start, count * sizeof(uint32_t));
+        for (uint32_t i = 0; i < count && ok; i++)
+            ok = words[i] == check_pattern(i, pass);
+    }
+    dcache_lines(&SCB_DCCIMVAC, start, count * sizeof(uint32_t));
+    return ok;
 }
 
 dmod_dmfmc_port_api_declaration(1.0, int, _init, ( void ))
@@ -248,6 +303,30 @@ dmod_dmfmc_port_api_declaration(1.0, int, _init, ( void ))
     return 0;
 }
 
+dmod_dmfmc_port_api_declaration(1.0, int, _cache_sdram, ( dmfmc_sdram_bank_t bank, void *start, uint32_t size ))
+{
+    (void)bank;
+    if (MPU_TYPE == 0U || start == NULL)
+        return 0; /* No MPU: the window keeps the default map's attributes */
+
+    set_sdram_attributes(MPU_RASR_WRITE_BACK);
+    if ((SCB_CCR & SCB_CCR_DC) == 0U)
+    {
+        DMOD_LOG_INFO("FMC: SDRAM is cacheable (write-back) - the data cache is off\n");
+        return 0;
+    }
+    if (!check_through_cache(start, size))
+    {
+        /* Nothing of it may stay in the cache once the window is uncached */
+        dcache_lines(&SCB_DCIMVAC, start, (size < CACHE_CHECK_SIZE) ? size : CACHE_CHECK_SIZE);
+        set_sdram_attributes(MPU_RASR_NONCACHEABLE);
+        DMOD_LOG_ERROR("FMC: SDRAM does not hold data written through the cache\n");
+        return -EIO;
+    }
+    DMOD_LOG_INFO("FMC: SDRAM is cached (write-back)\n");
+    return 0;
+}
+
 dmod_dmfmc_port_api_declaration(1.0, int, _deinit, ( void ))
 {
     for (int i = 0; i < 2; i++)