Merge pull request #3028 from cleanflight/improve-osd-slave-comms (#5405)

Improve OSD slave timeout/disconnect/reconnect handling.

src/main/drivers/display.c

@@ -92,6 +92,11 @@ bool displayIsTransferInProgress(const displayPort_t *instance)
     return instance->vTable->isTransferInProgress(instance);
 }
 
+bool displayIsSynced(const displayPort_t *instance)
+{
+    return instance->vTable->isSynced(instance);
+}
+
 void displayHeartbeat(displayPort_t *instance)
 {
     instance->vTable->heartbeat(instance);

src/main/drivers/display.h

@@ -46,6 +46,7 @@ typedef struct displayPortVTable_s {
     bool (*isTransferInProgress)(const displayPort_t *displayPort);
     int (*heartbeat)(displayPort_t *displayPort);
     void (*resync)(displayPort_t *displayPort);
+    bool (*isSynced)(const displayPort_t *displayPort);
     uint32_t (*txBytesFree)(const displayPort_t *displayPort);
 } displayPortVTable_t;
 
@@ -70,5 +71,6 @@ int displayWriteChar(displayPort_t *instance, uint8_t x, uint8_t y, uint8_t c);
 bool displayIsTransferInProgress(const displayPort_t *instance);
 void displayHeartbeat(displayPort_t *instance);
 void displayResync(displayPort_t *instance);
+bool displayIsSynced(const displayPort_t *instance);
 uint16_t displayTxBytesFree(const displayPort_t *instance);
 void displayInit(displayPort_t *instance, const displayPortVTable_t *vTable);

src/main/drivers/max7456.c

@@ -220,6 +220,7 @@ static IO_t max7456CsPin        = IO_NONE;
 
 static uint8_t max7456DeviceType;
 
+static void max7456DrawScreenSlow(void);
 
 static uint8_t max7456Send(uint8_t add, uint8_t data)
 {
@@ -400,7 +401,7 @@ void max7456ReInit(void)
     // Clear shadow to force redraw all screen in non-dma mode.
     memset(shadowBuffer, 0, maxScreenSize);
     if (firstInit) {
-        max7456RefreshAll();
+        max7456DrawScreenSlow();
         firstInit = false;
     }
 }
@@ -543,62 +544,77 @@ bool max7456DmaInProgress(void)
 #endif
 }
 
-void max7456DrawScreen(void)
+bool max7456BuffersSynced(void)
+{
+    for (int i = 0; i < maxScreenSize; i++) {
+        if (screenBuffer[i] != shadowBuffer[i]) {
+            return false;
+        }
+    }
+    return true;
+}
+
+void max7456ReInitIfRequired(void)
 {
     static uint32_t lastSigCheckMs = 0;
     static uint32_t videoDetectTimeMs = 0;
-    static uint16_t pos = 0;
-
     static uint16_t reInitCount = 0;
 
+    ENABLE_MAX7456;
+    const uint8_t stallCheck = max7456Send(MAX7456ADD_VM0|MAX7456ADD_READ, 0x00);
+    DISABLE_MAX7456;
+
+    const timeMs_t nowMs = millis();
+
+    if (stallCheck != videoSignalReg) {
+        max7456ReInit();
+    } else if ((videoSignalCfg == VIDEO_SYSTEM_AUTO)
+              && ((nowMs - lastSigCheckMs) > MAX7456_SIGNAL_CHECK_INTERVAL_MS)) {
+
+        // Adjust output format based on the current input format.
+
+        ENABLE_MAX7456;
+        const uint8_t videoSense = max7456Send(MAX7456ADD_STAT, 0x00);
+        DISABLE_MAX7456;
+
+        DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_MODEREG, videoSignalReg & VIDEO_MODE_MASK);
+        DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_SENSE, videoSense & 0x7);
+        DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_ROWS, max7456GetRowsCount());
+
+        if (videoSense & STAT_LOS) {
+            videoDetectTimeMs = 0;
+        } else {
+            if ((VIN_IS_PAL(videoSense) && VIDEO_MODE_IS_NTSC(videoSignalReg))
+              || (VIN_IS_NTSC_alt(videoSense) && VIDEO_MODE_IS_PAL(videoSignalReg))) {
+                if (videoDetectTimeMs) {
+                    if (millis() - videoDetectTimeMs > VIDEO_SIGNAL_DEBOUNCE_MS) {
+                        max7456ReInit();
+                        DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_REINIT, ++reInitCount);
+                    }
+                } else {
+                    // Wait for signal to stabilize
+                    videoDetectTimeMs = millis();
+                }
+            }
+        }
+
+        lastSigCheckMs = nowMs;
+    }
+
+    //------------   end of (re)init-------------------------------------
+}
+
+void max7456DrawScreen(void)
+{
+    static uint16_t pos = 0;
+
     if (!max7456Lock && !fontIsLoading) {
 
         // (Re)Initialize MAX7456 at startup or stall is detected.
 
         max7456Lock = true;
-        ENABLE_MAX7456;
-        const uint8_t stallCheck = max7456Send(MAX7456ADD_VM0|MAX7456ADD_READ, 0x00);
-        DISABLE_MAX7456;
 
-        const timeMs_t nowMs = millis();
+        max7456ReInitIfRequired();
-
-        if (stallCheck != videoSignalReg) {
-            max7456ReInit();
-
-        } else if ((videoSignalCfg == VIDEO_SYSTEM_AUTO)
-                  && ((nowMs - lastSigCheckMs) > MAX7456_SIGNAL_CHECK_INTERVAL_MS)) {
-
-            // Adjust output format based on the current input format.
-
-            ENABLE_MAX7456;
-            const uint8_t videoSense = max7456Send(MAX7456ADD_STAT, 0x00);
-            DISABLE_MAX7456;
-
-            DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_MODEREG, videoSignalReg & VIDEO_MODE_MASK);
-            DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_SENSE, videoSense & 0x7);
-            DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_ROWS, max7456GetRowsCount());
-
-            if (videoSense & STAT_LOS) {
-                videoDetectTimeMs = 0;
-            } else {
-                if ((VIN_IS_PAL(videoSense) && VIDEO_MODE_IS_NTSC(videoSignalReg))
-                  || (VIN_IS_NTSC_alt(videoSense) && VIDEO_MODE_IS_PAL(videoSignalReg))) {
-                    if (videoDetectTimeMs) {
-                        if (millis() - videoDetectTimeMs > VIDEO_SIGNAL_DEBOUNCE_MS) {
-                            max7456ReInit();
-                            DEBUG_SET(DEBUG_MAX7456_SIGNAL, DEBUG_MAX7456_SIGNAL_REINIT, ++reInitCount);
-                        }
-                    } else {
-                        // Wait for signal to stabilize
-                        videoDetectTimeMs = millis();
-                    }
-                }
-            }
-
-            lastSigCheckMs = nowMs;
-        }
-
-        //------------   end of (re)init-------------------------------------
 
         int buff_len = 0;
         for (int k = 0; k < MAX_CHARS2UPDATE; k++) {
@@ -632,8 +648,25 @@ void max7456DrawScreen(void)
     }
 }
 
-// This funcktion refresh all and should not be used when copter is armed
+static void max7456DrawScreenSlow(void)
+{
+    ENABLE_MAX7456;
+    max7456Send(MAX7456ADD_DMAH, 0);
+    max7456Send(MAX7456ADD_DMAL, 0);
+    max7456Send(MAX7456ADD_DMM, displayMemoryModeReg | 1);
 
+    for (int xx = 0; xx < maxScreenSize; ++xx) {
+        max7456Send(MAX7456ADD_DMDI, screenBuffer[xx]);
+        shadowBuffer[xx] = screenBuffer[xx];
+    }
+
+    max7456Send(MAX7456ADD_DMDI, 0xFF);
+    max7456Send(MAX7456ADD_DMM, displayMemoryModeReg);
+    DISABLE_MAX7456;
+}
+
+
+// should not be used when armed
 void max7456RefreshAll(void)
 {
     if (!max7456Lock) {
@@ -641,19 +674,10 @@ void max7456RefreshAll(void)
         while (dmaTransactionInProgress);
 #endif
         max7456Lock = true;
-        ENABLE_MAX7456;
-        max7456Send(MAX7456ADD_DMAH, 0);
-        max7456Send(MAX7456ADD_DMAL, 0);
-        max7456Send(MAX7456ADD_DMM, displayMemoryModeReg | 1);
 
-        for (int xx = 0; xx < maxScreenSize; ++xx) {
+        max7456ReInitIfRequired();
-            max7456Send(MAX7456ADD_DMDI, screenBuffer[xx]);
+        max7456DrawScreenSlow();
-            shadowBuffer[xx] = screenBuffer[xx];
-        }
 
-        max7456Send(MAX7456ADD_DMDI, 0xFF);
-        max7456Send(MAX7456ADD_DMM, displayMemoryModeReg);
-        DISABLE_MAX7456;
         max7456Lock = false;
     }
 }

src/main/drivers/max7456.h

@@ -40,3 +40,4 @@ void    max7456ClearScreen(void);
 void    max7456RefreshAll(void);
 uint8_t* max7456GetScreenBuffer(void);
 bool    max7456DmaInProgress(void);
+bool    max7456BuffersSynced(void);

src/main/io/displayport_max7456.c

@@ -120,6 +120,12 @@ static bool isTransferInProgress(const displayPort_t *displayPort)
     return max7456DmaInProgress();
 }
 
+static bool isSynced(const displayPort_t *displayPort)
+{
+    UNUSED(displayPort);
+    return max7456BuffersSynced();
+}
+
 static void resync(displayPort_t *displayPort)
 {
     UNUSED(displayPort);
@@ -151,6 +157,7 @@ static const displayPortVTable_t max7456VTable = {
     .isTransferInProgress = isTransferInProgress,
     .heartbeat = heartbeat,
     .resync = resync,
+    .isSynced = isSynced,
     .txBytesFree = txBytesFree,
 };
 

src/main/io/displayport_msp.c

@@ -134,10 +134,17 @@ static bool isTransferInProgress(const displayPort_t *displayPort)
     return false;
 }
 
+static bool isSynced(const displayPort_t *displayPort)
+{
+    UNUSED(displayPort);
+    return true;
+}
+
 static void resync(displayPort_t *displayPort)
 {
     displayPort->rows = 13 + displayPortProfileMsp()->rowAdjust; // XXX Will reflect NTSC/PAL in the future
     displayPort->cols = 30 + displayPortProfileMsp()->colAdjust;
+    drawScreen(displayPort);
 }
 
 static uint32_t txBytesFree(const displayPort_t *displayPort)
@@ -157,6 +164,7 @@ static const displayPortVTable_t mspDisplayPortVTable = {
     .isTransferInProgress = isTransferInProgress,
     .heartbeat = heartbeat,
     .resync = resync,
+    .isSynced = isSynced,
     .txBytesFree = txBytesFree
 };
 

src/main/io/displayport_oled.c

@@ -76,6 +76,12 @@ static bool oledIsTransferInProgress(const displayPort_t *displayPort)
     return false;
 }
 
+static bool oledIsSynced(const displayPort_t *displayPort)
+{
+    UNUSED(displayPort);
+    return true;
+}
+
 static int oledHeartbeat(displayPort_t *displayPort)
 {
     UNUSED(displayPort);
@@ -104,6 +110,7 @@ static const displayPortVTable_t oledVTable = {
     .isTransferInProgress = oledIsTransferInProgress,
     .heartbeat = oledHeartbeat,
     .resync = oledResync,
+    .isSynced = oledIsSynced,
     .txBytesFree = oledTxBytesFree
 };
 

src/main/io/displayport_rcdevice.c

@@ -43,6 +43,7 @@ static const displayPortVTable_t rcdeviceOSDVTable = {
     .isTransferInProgress = rcdeviceOSDIsTransferInProgress,
     .heartbeat = rcdeviceOSDHeartbeat,
     .resync = rcdeviceOSDResync,
+    .isSynced = rcdeviceOSDIsSynced,
     .txBytesFree = rcdeviceOSDTxBytesFree,
     .screenSize = rcdeviceScreenSize,
 };

src/main/io/displayport_srxl.c

@@ -80,6 +80,12 @@ static bool srxlIsTransferInProgress(const displayPort_t *displayPort)
     return false;
 }
 
+static bool srxlIsSynced(const displayPort_t *displayPort)
+{
+    UNUSED(displayPort);
+    return true;
+}
+
 static int srxlHeartbeat(displayPort_t *displayPort)
 {
     UNUSED(displayPort);
@@ -120,6 +126,7 @@ static const displayPortVTable_t srxlVTable = {
     .isTransferInProgress = srxlIsTransferInProgress,
     .heartbeat = srxlHeartbeat,
     .resync = srxlResync,
+    .isSynced = srxlIsSynced,
     .txBytesFree = srxlTxBytesFree
 };
 

src/main/io/osd_slave.c

@@ -86,7 +86,11 @@ void osdSlaveWrite(const uint8_t x, const uint8_t y, const char *s)
 void osdSlaveHeartbeat(void)
 {
     timeoutAt = micros() + (1000 * 1000);
-    stalled = false;
+    if (stalled) {
+        stalled = false;
+
+        displayResync(osdDisplayPort);
+    }
 }
 
 void osdSlaveInit(displayPort_t *osdDisplayPortToUse)
@@ -96,7 +100,12 @@ void osdSlaveInit(displayPort_t *osdDisplayPortToUse)
 
     osdDisplayPort = osdDisplayPortToUse;
 
+    delay(100); // need max7456 to be ready before using the displayPort API further.
+
     displayClearScreen(osdDisplayPort);
+    displayResync(osdDisplayPort);
+
+    delay(100); // wait a little for video to stabilise
 
     osdDrawLogo(3, 1);
 
@@ -106,8 +115,6 @@ void osdSlaveInit(displayPort_t *osdDisplayPortToUse)
     displayWrite(osdDisplayPort, 13, 6, "OSD");
 
     displayResync(osdDisplayPort);
-
-    displayDrawScreenQueued = true;
 }
 
 bool osdSlaveCheck(timeUs_t currentTimeUs, timeDelta_t currentDeltaTimeUs)
@@ -122,7 +129,12 @@ bool osdSlaveCheck(timeUs_t currentTimeUs, timeDelta_t currentDeltaTimeUs)
         displayResync(osdDisplayPort);
     }
 
-    return receivingScreen || displayDrawScreenQueued || stalled;
+    if (!receivingScreen && !displayIsSynced(osdDisplayPort)) {
+        // queue a screen draw to ensure any remaining characters not written to the screen yet
+        // remember that displayDrawScreen() may return WITHOUT having fully updated the screen.
+        displayDrawScreenQueued = true;
+    }
+    return receivingScreen || displayDrawScreenQueued;
 }
 
 /*
@@ -147,7 +159,7 @@ void osdSlaveUpdate(timeUs_t currentTimeUs)
     }
 #endif
 
-    if (displayDrawScreenQueued || stalled) {
+    if (displayDrawScreenQueued) {
         displayDrawScreen(osdDisplayPort);
         displayDrawScreenQueued = false;
         receivingScreen = false;

src/main/io/rcdevice_osd.c

@@ -204,6 +204,12 @@ bool rcdeviceOSDIsTransferInProgress(const displayPort_t *displayPort)
     return false;
 }
 
+bool rcdeviceOSDIsSynced(const displayPort_t *displayPort)
+{
+    UNUSED(displayPort);
+    return true;
+}
+
 int rcdeviceOSDHeartbeat(displayPort_t *displayPort)
 {
     UNUSED(displayPort);

src/main/io/rcdevice_osd.h

@@ -35,5 +35,6 @@ int rcdeviceOSDClearScreen(displayPort_t *);
 bool rcdeviceOSDIsTransferInProgress(const displayPort_t *);
 int rcdeviceOSDHeartbeat(displayPort_t *displayPort);
 void rcdeviceOSDResync(displayPort_t *displayPort);
+bool rcdeviceOSDIsSynced(const displayPort_t *displayPort);
 uint32_t rcdeviceOSDTxBytesFree(const displayPort_t *displayPort);
 int rcdeviceScreenSize(const displayPort_t *displayPort);