浏览代码

Especially when internal memory fills up, some FreeRTOS structures (queues etc) get allocated in psram. These structures also contain a spinlock, which needs an atomic-compare-swap operation to work. The psram hardware, however, does not support this operation. As a workaround, this patch detects these spinlocks and will, instead of S32C1I, use equivalent C-code to simulate the behaviour, with an (internal) mux for atomicity.

Jeroen Domburg 8 年之前
父节点
当前提交
70ab924dbb

+ 2 - 0
components/freertos/include/freertos/portable.h

@@ -219,5 +219,7 @@ uint32_t xPortGetTickRateHz(void);
 }
 #endif
 
+void uxPortCompareSetExtram(volatile uint32_t *addr, uint32_t compare, uint32_t *set);
+
 #endif /* PORTABLE_H */
 

+ 28 - 4
components/freertos/port.c

@@ -306,10 +306,6 @@ void vPortAssertIfInISR()
  * For kernel use: Initialize a per-CPU mux. Mux will be initialized unlocked.
  */
 void vPortCPUInitializeMutex(portMUX_TYPE *mux) {
-#if defined(CONFIG_SPIRAM_SUPPORT)
-    // Check if mux belongs to internal memory (DRAM), prerequisite for atomic operations
-    configASSERT(esp_ptr_internal((const void *) mux));
-#endif
 
 #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
 	ets_printf("Initializing mux %p\n", mux);
@@ -386,6 +382,34 @@ void vPortSetStackWatchpoint( void* pxStackStart ) {
 	esp_set_watchpoint(1, (char*)addr, 32, ESP_WATCHPOINT_STORE);
 }
 
+#if defined(CONFIG_SPIRAM_SUPPORT)
+/*
+ * Compare & set (S32C1) does not work in external RAM. Instead, this routine uses a mux (in internal memory) to fake it.
+ */
+static portMUX_TYPE extram_mux = portMUX_INITIALIZER_UNLOCKED;
+
+void uxPortCompareSetExtram(volatile uint32_t *addr, uint32_t compare, uint32_t *set) {
+	uint32_t prev;
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+	vPortCPUAcquireMutexIntsDisabled(&extram_mux, portMUX_NO_TIMEOUT, __FUNCTION__, __LINE__);
+#else
+	vPortCPUAcquireMutexIntsDisabled(&extram_mux, portMUX_NO_TIMEOUT); 
+#endif
+	prev=*addr;
+	if (prev==compare) {
+		*addr=*set;
+	}
+	*set=prev;
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+	vPortCPUReleaseMutexIntsDisabled(&extram_mux, __FUNCTION__, __LINE__);
+#else
+	vPortCPUReleaseMutexIntsDisabled(&extram_mux);
+#endif
+}
+#endif //defined(CONFIG_SPIRAM_SUPPORT)
+
+
+
 uint32_t xPortGetTickRateHz(void) {
 	return (uint32_t)configTICK_RATE_HZ;
 }

+ 52 - 110
components/freertos/portmux_impl.h

@@ -32,138 +32,80 @@
    deal by FreeRTOS internals.
 
    It should be #included by freertos port.c or tasks.c, in esp-idf.
+
+   The way it works is that it essentially uses portmux_impl.inc.h as a
+   generator template of sorts. When no external memory is used, this 
+   template is only used to generate the vPortCPUAcquireMutexIntsDisabledInternal
+   and vPortCPUReleaseMutexIntsDisabledInternal functions, which use S32C1 to
+   do an atomic compare & swap. When external memory is used the functions
+   vPortCPUAcquireMutexIntsDisabledExtram and vPortCPUReleaseMutexIntsDisabledExtram
+   are also generated, which use uxPortCompareSetExtram to fake the S32C1 instruction.
+   The wrapper functions vPortCPUAcquireMutexIntsDisabled and 
+   vPortCPUReleaseMutexIntsDisabled will then use the appropriate function to do the
+   actual lock/unlock.
 */
 #include "soc/cpu.h"
+#include "portable.h"
 
 /* XOR one core ID with this value to get the other core ID */
 #define CORE_ID_XOR_SWAP (CORE_ID_PRO ^ CORE_ID_APP)
 
-static inline bool __attribute__((always_inline))
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
-vPortCPUAcquireMutexIntsDisabled(portMUX_TYPE *mux, int timeout_cycles, const char *fnName, int line) {
-#else
-vPortCPUAcquireMutexIntsDisabled(portMUX_TYPE *mux, int timeout_cycles) {
-#endif
-#if !CONFIG_FREERTOS_UNICORE
-	uint32_t res;
-	portBASE_TYPE coreID, otherCoreID;
-	uint32_t ccount_start;
-	bool set_timeout = timeout_cycles > portMUX_NO_TIMEOUT;
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
-	if (!set_timeout) {
-		timeout_cycles = 10000; // Always set a timeout in debug mode
-		set_timeout = true;
-	}
-#endif
-	if (set_timeout) { // Timeout
-		RSR(CCOUNT, ccount_start);
-	}
 
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
-	uint32_t owner = mux->owner;
-	if (owner != portMUX_FREE_VAL && owner != CORE_ID_PRO && owner != CORE_ID_APP) {
-		ets_printf("ERROR: vPortCPUAcquireMutex: mux %p is uninitialized (0x%X)! Called from %s line %d.\n", mux, owner, fnName, line);
-		mux->owner=portMUX_FREE_VAL;
-	}
-#endif
 
-	/* Spin until we own the core */
-
-	RSR(PRID, coreID);
-	/* Note: coreID is the full 32 bit core ID (CORE_ID_PRO/CORE_ID_APP),
-	   not the 0/1 value returned by xPortGetCoreID()
-	*/
-	otherCoreID = CORE_ID_XOR_SWAP ^ coreID;
-	do {
-		/* mux->owner should be one of portMUX_FREE_VAL, CORE_ID_PRO,
-		   CORE_ID_APP:
-
-		   - If portMUX_FREE_VAL, we want to atomically set to 'coreID'.
-		   - If "our" coreID, we can drop through immediately.
-		   - If "otherCoreID", we spin here.
-		 */
-		res = coreID;
-		uxPortCompareSet(&mux->owner, portMUX_FREE_VAL, &res);
-
-		if (res != otherCoreID) {
-			break; // mux->owner is "our" coreID
-		}
-
-		if (set_timeout) {
-			uint32_t ccount_now;
-			RSR(CCOUNT, ccount_now);
-			if (ccount_now - ccount_start > (unsigned)timeout_cycles) {
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
-				ets_printf("Timeout on mux! last non-recursive lock %s line %d, curr %s line %d\n", mux->lastLockedFn, mux->lastLockedLine, fnName, line);
-				ets_printf("Owner 0x%x count %d\n", mux->owner, mux->count);
-#endif
-				return false;
-			}
-		}
-	} while (1);
 
-	assert(res == coreID || res == portMUX_FREE_VAL); /* any other value implies memory corruption or uninitialized mux */
-	assert((res == portMUX_FREE_VAL) == (mux->count == 0)); /* we're first to lock iff count is zero */
-    assert(mux->count < 0xFF); /* Bad count value implies memory corruption */
+//Define the mux routines for use with muxes in internal RAM
+#define PORTMUX_AQUIRE_MUX_FN_NAME vPortCPUAcquireMutexIntsDisabledInternal
+#define PORTMUX_RELEASE_MUX_FN_NAME vPortCPUReleaseMutexIntsDisabledInternal
+#define PORTMUX_COMPARE_SET_FN_NAME uxPortCompareSet
+#include "portmux_impl.inc.h"
+#undef PORTMUX_AQUIRE_MUX_FN_NAME
+#undef PORTMUX_RELEASE_MUX_FN_NAME
+#undef PORTMUX_COMPARE_SET_FN_NAME
 
-	/* now we own it, we can increment the refcount */
-	mux->count++;
 
+#if defined(CONFIG_SPIRAM_SUPPORT)
 
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
-	if (res==portMUX_FREE_VAL) { //initial lock
-		mux->lastLockedFn=fnName;
-		mux->lastLockedLine=line;
-	} else {
-		ets_printf("Recursive lock: count=%d last non-recursive lock %s line %d, curr %s line %d\n", mux->count-1,
-				   mux->lastLockedFn, mux->lastLockedLine, fnName, line);
-	}
-#endif /* CONFIG_FREERTOS_PORTMUX_DEBUG */
-#endif /* CONFIG_FREERTOS_UNICORE */
-	return true;
-}
+#define PORTMUX_AQUIRE_MUX_FN_NAME vPortCPUAcquireMutexIntsDisabledExtram
+#define PORTMUX_RELEASE_MUX_FN_NAME vPortCPUReleaseMutexIntsDisabledExtram
+#define PORTMUX_COMPARE_SET_FN_NAME uxPortCompareSetExtram
+#include "portmux_impl.inc.h"
+#undef PORTMUX_AQUIRE_MUX_FN_NAME
+#undef PORTMUX_RELEASE_MUX_FN_NAME
+#undef PORTMUX_COMPARE_SET_FN_NAME
 
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
- static inline void vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux, const char *fnName, int line) {
-#else
-static inline void vPortCPUReleaseMutexIntsDisabled(portMUX_TYPE *mux) {
 #endif
-#if !CONFIG_FREERTOS_UNICORE
-	portBASE_TYPE coreID;
+
+
 #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
-	const char *lastLockedFn=mux->lastLockedFn;
-	int lastLockedLine=mux->lastLockedLine;
-	mux->lastLockedFn=fnName;
-	mux->lastLockedLine=line;
-	uint32_t owner = mux->owner;
-	if (owner != portMUX_FREE_VAL && owner != CORE_ID_PRO && owner != CORE_ID_APP) {
-		ets_printf("ERROR: vPortCPUReleaseMutex: mux %p is invalid (0x%x)!\n", mux, mux->owner);
-	}
+#define PORTMUX_AQUIRE_MUX_FN_ARGS portMUX_TYPE *mux, int timeout_cycles, const char *fnName, int line
+#define PORTMUX_RELEASE_MUX_FN_ARGS portMUX_TYPE *mux, const char *fnName, int line
+#define PORTMUX_AQUIRE_MUX_FN_CALL_ARGS(x) x, timeout_cycles, fnName, line
+#define PORTMUX_RELEASE_MUX_FN_CALL_ARGS(x) x, fnName, line
+#else
+#define PORTMUX_AQUIRE_MUX_FN_ARGS portMUX_TYPE *mux, int timeout_cycles
+#define PORTMUX_RELEASE_MUX_FN_ARGS portMUX_TYPE *mux
+#define PORTMUX_AQUIRE_MUX_FN_CALL_ARGS(x) x, timeout_cycles
+#define PORTMUX_RELEASE_MUX_FN_CALL_ARGS(x) x
 #endif
 
-#if CONFIG_FREERTOS_PORTMUX_DEBUG || !defined(NDEBUG)
-	RSR(PRID, coreID);
-#endif
 
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
-	if (coreID != mux->owner) {
-		ets_printf("ERROR: vPortCPUReleaseMutex: mux %p was already unlocked!\n", mux);
-		ets_printf("Last non-recursive unlock %s line %d, curr unlock %s line %d\n", lastLockedFn, lastLockedLine, fnName, line);
+static inline bool __attribute__((always_inline)) vPortCPUAcquireMutexIntsDisabled(PORTMUX_AQUIRE_MUX_FN_ARGS) {
+#if defined(CONFIG_SPIRAM_SUPPORT)
+	if (esp_ptr_external_ram(mux)) {
+		return vPortCPUAcquireMutexIntsDisabledExtram(PORTMUX_AQUIRE_MUX_FN_CALL_ARGS(mux));
 	}
 #endif
+	return vPortCPUAcquireMutexIntsDisabledInternal(PORTMUX_AQUIRE_MUX_FN_CALL_ARGS(mux));
+}
 
-    assert(coreID == mux->owner); // This is a mutex we didn't lock, or it's corrupt
-    assert(mux->count > 0); // Indicates memory corruption
-    assert(mux->count < 0x100); // Indicates memory corruption
 
-	mux->count--;
-	if(mux->count == 0) {
-		mux->owner = portMUX_FREE_VAL;
-	}
-#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG_RECURSIVE
-	else {
-		ets_printf("Recursive unlock: count=%d last locked %s line %d, curr %s line %d\n", mux->count, lastLockedFn, lastLockedLine, fnName, line);
+static inline void vPortCPUReleaseMutexIntsDisabled(PORTMUX_RELEASE_MUX_FN_ARGS) {
+#if defined(CONFIG_SPIRAM_SUPPORT)
+	if (esp_ptr_external_ram(mux)) {
+		vPortCPUReleaseMutexIntsDisabledExtram(PORTMUX_RELEASE_MUX_FN_CALL_ARGS(mux));
+		return;
 	}
 #endif
-#endif //!CONFIG_FREERTOS_UNICORE
+	vPortCPUReleaseMutexIntsDisabledInternal(PORTMUX_RELEASE_MUX_FN_CALL_ARGS(mux));
 }
+

+ 171 - 0
components/freertos/portmux_impl.inc.h

@@ -0,0 +1,171 @@
+/*
+    Copyright (C) 2016-2017 Espressif Shanghai PTE LTD
+    Copyright (C) 2015 Real Time Engineers Ltd.
+
+    All rights reserved
+
+    FreeRTOS is free software; you can redistribute it and/or modify it under
+    the terms of the GNU General Public License (version 2) as published by the
+    Free Software Foundation >>!AND MODIFIED BY!<< the FreeRTOS exception.
+
+	***************************************************************************
+    >>!   NOTE: The modification to the GPL is included to allow you to     !<<
+    >>!   distribute a combined work that includes FreeRTOS without being   !<<
+    >>!   obliged to provide the source code for proprietary components     !<<
+    >>!   outside of the FreeRTOS kernel.                                   !<<
+	***************************************************************************
+
+    FreeRTOS is distributed in the hope that it will be useful, but WITHOUT ANY
+    WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+    FOR A PARTICULAR PURPOSE.  Full license text is available on the following
+    link: http://www.freertos.org/a00114.html
+*/
+
+
+/*
+  Warning: funky preprocessor hackery ahead. Including these headers will generate two
+  functions, which names are defined by the preprocessor macros 
+  PORTMUX_AQUIRE_MUX_FN_NAME and PORTMUX_RELEASE_MUX_FN_NAME. In order to do the compare
+  and exchange function, they will use whatever PORTMUX_COMPARE_SET_FN_NAME resolves to.
+
+  In some scenarios, this header is included *twice* in portmux_impl.h: one time 
+  for the 'normal' mux code which uses a compare&exchange routine, another time 
+  to generate code for a second set of these routines that use a second mux 
+  (in internal ram) to fake a compare&exchange on a variable in external memory.
+*/
+
+
+
+static inline bool __attribute__((always_inline))
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+PORTMUX_AQUIRE_MUX_FN_NAME(portMUX_TYPE *mux, int timeout_cycles, const char *fnName, int line) {
+#else
+PORTMUX_AQUIRE_MUX_FN_NAME(portMUX_TYPE *mux, int timeout_cycles) {
+#endif
+
+
+#if !CONFIG_FREERTOS_UNICORE
+	uint32_t res;
+	portBASE_TYPE coreID, otherCoreID;
+	uint32_t ccount_start;
+	bool set_timeout = timeout_cycles > portMUX_NO_TIMEOUT;
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+	if (!set_timeout) {
+		timeout_cycles = 10000; // Always set a timeout in debug mode
+		set_timeout = true;
+	}
+#endif
+	if (set_timeout) { // Timeout
+		RSR(CCOUNT, ccount_start);
+	}
+
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+	uint32_t owner = mux->owner;
+	if (owner != portMUX_FREE_VAL && owner != CORE_ID_PRO && owner != CORE_ID_APP) {
+		ets_printf("ERROR: vPortCPUAcquireMutex: mux %p is uninitialized (0x%X)! Called from %s line %d.\n", mux, owner, fnName, line);
+		mux->owner=portMUX_FREE_VAL;
+	}
+#endif
+
+	/* Spin until we own the core */
+
+	RSR(PRID, coreID);
+	/* Note: coreID is the full 32 bit core ID (CORE_ID_PRO/CORE_ID_APP),
+	   not the 0/1 value returned by xPortGetCoreID()
+	*/
+	otherCoreID = CORE_ID_XOR_SWAP ^ coreID;
+	do {
+		/* mux->owner should be one of portMUX_FREE_VAL, CORE_ID_PRO,
+		   CORE_ID_APP:
+
+		   - If portMUX_FREE_VAL, we want to atomically set to 'coreID'.
+		   - If "our" coreID, we can drop through immediately.
+		   - If "otherCoreID", we spin here.
+		 */
+		res = coreID;
+		PORTMUX_COMPARE_SET_FN_NAME(&mux->owner, portMUX_FREE_VAL, &res);
+
+		if (res != otherCoreID) {
+			break; // mux->owner is "our" coreID
+		}
+
+		if (set_timeout) {
+			uint32_t ccount_now;
+			RSR(CCOUNT, ccount_now);
+			if (ccount_now - ccount_start > (unsigned)timeout_cycles) {
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+				ets_printf("Timeout on mux! last non-recursive lock %s line %d, curr %s line %d\n", mux->lastLockedFn, mux->lastLockedLine, fnName, line);
+				ets_printf("Owner 0x%x count %d\n", mux->owner, mux->count);
+#endif
+				return false;
+			}
+		}
+	} while (1);
+
+	assert(res == coreID || res == portMUX_FREE_VAL); /* any other value implies memory corruption or uninitialized mux */
+	assert((res == portMUX_FREE_VAL) == (mux->count == 0)); /* we're first to lock iff count is zero */
+    assert(mux->count < 0xFF); /* Bad count value implies memory corruption */
+
+	/* now we own it, we can increment the refcount */
+	mux->count++;
+
+
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+	if (res==portMUX_FREE_VAL) { //initial lock
+		mux->lastLockedFn=fnName;
+		mux->lastLockedLine=line;
+	} else {
+		ets_printf("Recursive lock: count=%d last non-recursive lock %s line %d, curr %s line %d\n", mux->count-1,
+				   mux->lastLockedFn, mux->lastLockedLine, fnName, line);
+	}
+#endif /* CONFIG_FREERTOS_PORTMUX_DEBUG */
+#endif /* CONFIG_FREERTOS_UNICORE */
+	return true;
+}
+
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+static inline void PORTMUX_RELEASE_MUX_FN_NAME(portMUX_TYPE *mux, const char *fnName, int line) {
+#else
+static inline void PORTMUX_RELEASE_MUX_FN_NAME(portMUX_TYPE *mux) {
+#endif
+
+
+#if !CONFIG_FREERTOS_UNICORE
+	portBASE_TYPE coreID;
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+	const char *lastLockedFn=mux->lastLockedFn;
+	int lastLockedLine=mux->lastLockedLine;
+	mux->lastLockedFn=fnName;
+	mux->lastLockedLine=line;
+	uint32_t owner = mux->owner;
+	if (owner != portMUX_FREE_VAL && owner != CORE_ID_PRO && owner != CORE_ID_APP) {
+		ets_printf("ERROR: vPortCPUReleaseMutex: mux %p is invalid (0x%x)!\n", mux, mux->owner);
+	}
+#endif
+
+#if CONFIG_FREERTOS_PORTMUX_DEBUG || !defined(NDEBUG)
+	RSR(PRID, coreID);
+#endif
+
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG
+	if (coreID != mux->owner) {
+		ets_printf("ERROR: vPortCPUReleaseMutex: mux %p was already unlocked!\n", mux);
+		ets_printf("Last non-recursive unlock %s line %d, curr unlock %s line %d\n", lastLockedFn, lastLockedLine, fnName, line);
+	}
+#endif
+
+	assert(coreID == mux->owner); // This is a mutex we didn't lock, or it's corrupt
+	assert(mux->count > 0); // Indicates memory corruption
+	assert(mux->count < 0x100); // Indicates memory corruption
+
+	mux->count--;
+	if(mux->count == 0) {
+		mux->owner = portMUX_FREE_VAL;
+	}
+#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG_RECURSIVE
+	else {
+		ets_printf("Recursive unlock: count=%d last locked %s line %d, curr %s line %d\n", mux->count, lastLockedFn, lastLockedLine, fnName, line);
+	}
+#endif
+#endif //!CONFIG_FREERTOS_UNICORE
+}

+ 4 - 0
components/freertos/test/test_spinlocks.c

@@ -47,9 +47,13 @@ TEST_CASE("portMUX spinlocks (no contention)", "[freertos]")
 
 #ifdef CONFIG_FREERTOS_UNICORE
     TEST_PERFORMANCE_LESS_THAN(FREERTOS_SPINLOCK_CYCLES_PER_OP_UNICORE, "%d cycles/op", ((end - start)/REPEAT_OPS));
+#else
+#if CONFIG_SPIRAM_SUPPORT
+    TEST_PERFORMANCE_LESS_THAN(FREERTOS_SPINLOCK_CYCLES_PER_OP_PSRAM, "%d cycles/op", ((end - start)/REPEAT_OPS));
 #else
     TEST_PERFORMANCE_LESS_THAN(FREERTOS_SPINLOCK_CYCLES_PER_OP, "%d cycles/op", ((end - start)/REPEAT_OPS));
 #endif
+#endif
 }
 
 TEST_CASE("portMUX recursive locks (no contention)", "[freertos]")

+ 1 - 0
components/idf_test/include/idf_performance.h

@@ -11,5 +11,6 @@
 /* declare the performance here */
 #define IDF_PERFORMANCE_MAX_HTTPS_REQUEST_BIN_SIZE                              800
 #define IDF_PERFORMANCE_MAX_FREERTOS_SPINLOCK_CYCLES_PER_OP                     200
+#define IDF_PERFORMANCE_MAX_FREERTOS_SPINLOCK_CYCLES_PER_OP_PSRAM               270
 #define IDF_PERFORMANCE_MAX_FREERTOS_SPINLOCK_CYCLES_PER_OP_UNICORE             130
 #define IDF_PERFORMANCE_MAX_ESP_TIMER_GET_TIME_PER_CALL                         1000

+ 0 - 1
components/soc/esp32/include/soc/soc.h

@@ -308,7 +308,6 @@
 #define SOC_MEM_INTERNAL_LOW        0x3FF90000
 #define SOC_MEM_INTERNAL_HIGH       0x400C2000
 
-
 //Interrupt hardware source table
 //This table is decided by hardware, don't touch this.
 #define ETS_WIFI_MAC_INTR_SOURCE                0/**< interrupt of WiFi MAC, level*/

+ 5 - 0
components/soc/include/soc/soc_memory_layout.h

@@ -89,3 +89,8 @@ inline static bool IRAM_ATTR esp_ptr_internal(const void *p) {
     r |= ((intptr_t)p >= SOC_RTC_DATA_LOW && (intptr_t)p < SOC_RTC_DATA_HIGH);
     return r;
 }
+
+
+inline static bool IRAM_ATTR esp_ptr_external_ram(const void *p) {
+    return ((intptr_t)p >= SOC_EXTRAM_DATA_LOW && (intptr_t)p < SOC_EXTRAM_DATA_HIGH);
+}