From d155dc0ed4906aefd282e0cf48629ff85f820fd9 Mon Sep 17 00:00:00 2001
From: Bruce Luckcuck <github@etracer.net>
Date: Sat, 26 Oct 2019 14:47:02 -0400
Subject: [PATCH] hange max7456 driver to only perform stall check once a
 second instead of every drawScreen call The max7456 driver performs a "stall
 check" looking to see if the device had stopped responding. It does this by
 reading the VM0 register and comparing it to the in-memory version.
 Presumably if communication failed because the device isn't responding then
 the result of the SPI transfer would be an unexpected value. If the incorrect
 value was returned then it would trigger a reinitialization in the hopes of
 getting the device to respond.

The problem is that this check was happening on **every** call to `displayDrawScreen` unnecessarily. So in the case of the OSD using the max7456 it would happen every 16.7ms (60hz) which is clearly overkill. The unnecessary register check was adding a fixed ~7us to every iteration of the OSD task (along with potential extra bus contention). So now the "stall test" is only performed once a second.
---
 src/main/drivers/max7456.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/main/drivers/max7456.c b/src/main/drivers/max7456.c
index 52d1b081d1..8dcfbb6980 100644
--- a/src/main/drivers/max7456.c
+++ b/src/main/drivers/max7456.c
@@ -119,6 +119,7 @@
 #define VIN_IS_NTSC_alt(val)  (!STAT_IS_LOS(val) && !STAT_IS_PAL(val))
 
 #define MAX7456_SIGNAL_CHECK_INTERVAL_MS 1000 // msec
+#define MAX7456_STALL_CHECK_INTERVAL_MS  1000 // msec
 
 // DMM special bits
 #define CLEAR_DISPLAY 0x04
@@ -613,19 +614,24 @@ bool max7456BuffersSynced(void)
     return true;
 }
 
-void max7456ReInitIfRequired(void)
+void max7456ReInitIfRequired(bool forceStallCheck)
 {
-    static uint32_t lastSigCheckMs = 0;
-    static uint32_t videoDetectTimeMs = 0;
+    static timeMs_t lastSigCheckMs = 0;
+    static timeMs_t videoDetectTimeMs = 0;
     static uint16_t reInitCount = 0;
-
-    __spiBusTransactionBegin(busdev);
-    const uint8_t stallCheck = max7456Send(MAX7456ADD_VM0|MAX7456ADD_READ, 0x00);
-    __spiBusTransactionEnd(busdev);
+    static timeMs_t lastStallCheckMs = MAX7456_STALL_CHECK_INTERVAL_MS / 2; // offset so that it doesn't coincide with the signal check
 
     const timeMs_t nowMs = millis();
 
-    if (stallCheck != videoSignalReg) {
+    bool stalled = false;
+    if (forceStallCheck || (lastStallCheckMs + MAX7456_STALL_CHECK_INTERVAL_MS < nowMs)) {
+        lastStallCheckMs = nowMs;
+        __spiBusTransactionBegin(busdev);
+        stalled = (max7456Send(MAX7456ADD_VM0|MAX7456ADD_READ, 0x00) != videoSignalReg);
+        __spiBusTransactionEnd(busdev);
+    }
+
+    if (stalled) {
         max7456ReInit();
     } else if ((videoSignalCfg == VIDEO_SYSTEM_AUTO)
               && ((nowMs - lastSigCheckMs) > MAX7456_SIGNAL_CHECK_INTERVAL_MS)) {
@@ -671,7 +677,7 @@ void max7456DrawScreen(void)
 
         // (Re)Initialize MAX7456 at startup or stall is detected.
 
-        max7456ReInitIfRequired();
+        max7456ReInitIfRequired(false);
 
         int buff_len = 0;
         for (int k = 0; k < MAX_CHARS2UPDATE; k++) {
@@ -751,7 +757,7 @@ void max7456RefreshAll(void)
     while (dmaTransactionInProgress);
 #endif
 
-    max7456ReInitIfRequired();
+    max7456ReInitIfRequired(true);
     max7456DrawScreenSlow();
 }