Include task header (Fixes sim)

src/FreeRTOSConfig.h

@@ -75,6 +75,7 @@
 #define configUSE_TIME_SLICING                  0
 #define configUSE_NEWLIB_REENTRANT              0
 #define configENABLE_BACKWARD_COMPATIBILITY     1
+#define configUSE_TASK_NOTIFICATIONS            0
 
 /* Hook function related definitions. */
 #define configUSE_IDLE_HOOK            0

src/displayapp/DisplayApp.cpp

@@ -142,9 +142,6 @@ void DisplayApp::Process(void* instance) {
   NRF_LOG_INFO("displayapp task started!");
   app->InitHw();
 
-  // Send a dummy notification to unlock the lvgl display driver for the first iteration
-  xTaskNotifyGive(xTaskGetCurrentTaskHandle());
-
   while (true) {
     app->Refresh();
   }

src/displayapp/DisplayAppRecovery.cpp

@@ -38,9 +38,6 @@ void DisplayApp::Process(void* instance) {
   auto* app = static_cast<DisplayApp*>(instance);
   NRF_LOG_INFO("displayapp task started!");
 
-  // Send a dummy notification to unlock the lvgl display driver for the first iteration
-  xTaskNotifyGive(xTaskGetCurrentTaskHandle());
-
   app->InitHw();
   while (true) {
     app->Refresh();
@@ -94,7 +91,6 @@ void DisplayApp::DisplayLogo(uint16_t color) {
   Pinetime::Tools::RleDecoder rleDecoder(infinitime_nb, sizeof(infinitime_nb), color, colorBlack);
   for (int i = 0; i < displayWidth; i++) {
     rleDecoder.DecodeNext(displayBuffer, displayWidth * bytesPerPixel);
-    ulTaskNotifyTake(pdTRUE, 500);
     lcd.DrawBuffer(0, i, displayWidth, 1, reinterpret_cast<const uint8_t*>(displayBuffer), displayWidth * bytesPerPixel);
   }
 }
@@ -103,7 +99,6 @@ void DisplayApp::DisplayOtaProgress(uint8_t percent, uint16_t color) {
   const uint8_t barHeight = 20;
   std::fill(displayBuffer, displayBuffer + (displayWidth * bytesPerPixel), color);
   for (int i = 0; i < barHeight; i++) {
-    ulTaskNotifyTake(pdTRUE, 500);
     uint16_t barWidth = std::min(static_cast<float>(percent) * 2.4f, static_cast<float>(displayWidth));
     lcd.DrawBuffer(0, displayWidth - barHeight + i, barWidth, 1, reinterpret_cast<const uint8_t*>(displayBuffer), barWidth * bytesPerPixel);
   }

src/displayapp/LittleVgl.cpp

@@ -152,10 +152,6 @@ void LittleVgl::SetFullRefresh(FullRefreshDirections direction) {
 void LittleVgl::FlushDisplay(const lv_area_t* area, lv_color_t* color_p) {
   uint16_t y1, y2, width, height = 0;
 
-  ulTaskNotifyTake(pdTRUE, 200);
-  // Notification is still needed (even if there is a mutex on SPI) because of the DataCommand pin
-  // which cannot be set/clear during a transfer.
-
   if ((scrollDirection == LittleVgl::FullRefreshDirections::Down) && (area->y2 == visibleNbLines - 1)) {
     writeOffset = ((writeOffset + totalNbLines) - visibleNbLines) % totalNbLines;
   } else if ((scrollDirection == FullRefreshDirections::Up) && (area->y1 == 0)) {
@@ -219,7 +215,6 @@ void LittleVgl::FlushDisplay(const lv_area_t* area, lv_color_t* color_p) {
 
     if (height > 0) {
       lcd.DrawBuffer(area->x1, y1, width, height, reinterpret_cast<const uint8_t*>(color_p), width * height * 2);
-      ulTaskNotifyTake(pdTRUE, 100);
     }
 
     uint16_t pixOffset = width * height;

src/drivers/Spi.cpp

@@ -9,8 +9,8 @@ Spi::Spi(SpiMaster& spiMaster, uint8_t pinCsn) : spiMaster {spiMaster}, pinCsn {
   nrf_gpio_pin_set(pinCsn);
 }
 
-bool Spi::Write(const uint8_t* data, size_t size, void (*TransactionHook)(bool)) {
+bool Spi::Write(const uint8_t* data, size_t size, const std::function<void()>& preTransactionHook) {
-  return spiMaster.Write(pinCsn, data, size, TransactionHook);
+  return spiMaster.Write(pinCsn, data, size, preTransactionHook);
 }
 
 bool Spi::Read(uint8_t* cmd, size_t cmdSize, uint8_t* data, size_t dataSize) {

src/drivers/Spi.h

@@ -1,6 +1,7 @@
 #pragma once
 #include <cstdint>
 #include <cstddef>
+#include <functional>
 #include "drivers/SpiMaster.h"
 
 namespace Pinetime {
@@ -14,7 +15,7 @@ namespace Pinetime {
       Spi& operator=(Spi&&) = delete;
 
       bool Init();
-      bool Write(const uint8_t* data, size_t size, void (*TransactionHook)(bool));
+      bool Write(const uint8_t* data, size_t size, const std::function<void()>& preTransactionHook);
       bool Read(uint8_t* cmd, size_t cmdSize, uint8_t* data, size_t dataSize);
       bool WriteCmdAndBuffer(const uint8_t* cmd, size_t cmdSize, const uint8_t* data, size_t dataSize);
       void Sleep();

src/drivers/SpiMaster.cpp

@@ -136,20 +136,11 @@ void SpiMaster::OnEndEvent() {
 
     spiBaseAddress->TASKS_START = 1;
   } else {
-    BaseType_t xHigherPriorityTaskWoken = pdFALSE;
-    if (taskToNotify != nullptr) {
-      vTaskNotifyGiveFromISR(taskToNotify, &xHigherPriorityTaskWoken);
-      portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
-    }
-
     nrf_gpio_pin_set(this->pinCsn);
-    if (this->TransactionHook != nullptr) {
-      this->TransactionHook(false);
-    }
     currentBufferAddr = 0;
-    BaseType_t xHigherPriorityTaskWoken2 = pdFALSE;
+    BaseType_t xHigherPriorityTaskWoken = pdFALSE;
-    xSemaphoreGiveFromISR(mutex, &xHigherPriorityTaskWoken2);
+    xSemaphoreGiveFromISR(mutex, &xHigherPriorityTaskWoken);
-    portYIELD_FROM_ISR(xHigherPriorityTaskWoken | xHigherPriorityTaskWoken2);
+    portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
   }
 }
 
@@ -176,14 +167,12 @@ void SpiMaster::PrepareRx(const uint32_t bufferAddress, const size_t size) {
   spiBaseAddress->EVENTS_END = 0;
 }
 
-bool SpiMaster::Write(uint8_t pinCsn, const uint8_t* data, size_t size, void (*TransactionHook)(bool)) {
+bool SpiMaster::Write(uint8_t pinCsn, const uint8_t* data, size_t size, const std::function<void()>& preTransactionHook) {
   if (data == nullptr)
     return false;
   auto ok = xSemaphoreTake(mutex, portMAX_DELAY);
   ASSERT(ok == true);
-  taskToNotify = xTaskGetCurrentTaskHandle();
 
-  this->TransactionHook = TransactionHook;
   this->pinCsn = pinCsn;
 
   if (size == 1) {
@@ -192,8 +181,8 @@ bool SpiMaster::Write(uint8_t pinCsn, const uint8_t* data, size_t size, void (*T
     DisableWorkaroundForFtpan58(spiBaseAddress, 0, 0);
   }
 
-  if (this->TransactionHook != nullptr) {
+  if (preTransactionHook != nullptr) {
-    this->TransactionHook(true);
+    preTransactionHook();
   }
   nrf_gpio_pin_clear(this->pinCsn);
 
@@ -210,9 +199,6 @@ bool SpiMaster::Write(uint8_t pinCsn, const uint8_t* data, size_t size, void (*T
     while (spiBaseAddress->EVENTS_END == 0)
       ;
     nrf_gpio_pin_set(this->pinCsn);
-    if (this->TransactionHook != nullptr) {
-      this->TransactionHook(false);
-    }
     currentBufferAddr = 0;
 
     DisableWorkaroundForFtpan58(spiBaseAddress, 0, 0);
@@ -226,8 +212,6 @@ bool SpiMaster::Write(uint8_t pinCsn, const uint8_t* data, size_t size, void (*T
 bool SpiMaster::Read(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t* data, size_t dataSize) {
   xSemaphoreTake(mutex, portMAX_DELAY);
 
-  taskToNotify = nullptr;
-  this->TransactionHook = nullptr;
   this->pinCsn = pinCsn;
   DisableWorkaroundForFtpan58(spiBaseAddress, 0, 0);
   spiBaseAddress->INTENCLR = (1 << 6);
@@ -275,10 +259,6 @@ void SpiMaster::Wakeup() {
 bool SpiMaster::WriteCmdAndBuffer(uint8_t pinCsn, const uint8_t* cmd, size_t cmdSize, const uint8_t* data, size_t dataSize) {
   xSemaphoreTake(mutex, portMAX_DELAY);
 
-  taskToNotify = nullptr;
-
-  this->TransactionHook = nullptr;
-
   this->pinCsn = pinCsn;
   DisableWorkaroundForFtpan58(spiBaseAddress, 0, 0);
   spiBaseAddress->INTENCLR = (1 << 6);

src/drivers/SpiMaster.h

@@ -1,6 +1,7 @@
 #pragma once
 #include <cstddef>
 #include <cstdint>
+#include <functional>
 
 #include <FreeRTOS.h>
 #include <semphr.h>
@@ -31,7 +32,7 @@ namespace Pinetime {
       SpiMaster& operator=(SpiMaster&&) = delete;
 
       bool Init();
-      bool Write(uint8_t pinCsn, const uint8_t* data, size_t size, void (*TransactionHook)(bool));
+      bool Write(uint8_t pinCsn, const uint8_t* data, size_t size, const std::function<void()>& preTransactionHook);
       bool Read(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t* data, size_t dataSize);
 
       bool WriteCmdAndBuffer(uint8_t pinCsn, const uint8_t* cmd, size_t cmdSize, const uint8_t* data, size_t dataSize);
@@ -50,14 +51,12 @@ namespace Pinetime {
 
       NRF_SPIM_Type* spiBaseAddress;
       uint8_t pinCsn;
-      void (*TransactionHook)(bool);
 
       SpiMaster::SpiModule spi;
       SpiMaster::Parameters params;
 
       volatile uint32_t currentBufferAddr = 0;
       volatile size_t currentBufferSize = 0;
-      volatile TaskHandle_t taskToNotify;
       SemaphoreHandle_t mutex = nullptr;
     };
   }

src/drivers/St7789.cpp

@@ -1,19 +1,18 @@
 #include "drivers/St7789.h"
 #include <hal/nrf_gpio.h>
-#include <libraries/delay/nrf_delay.h>
 #include <nrfx_log.h>
 #include "drivers/Spi.h"
-#include "drivers/PinMap.h"
+#include "task.h"
 
 using namespace Pinetime::Drivers;
 
-St7789::St7789(Spi& spi) : spi {spi} {
+St7789::St7789(Spi& spi, uint8_t pinDataCommand, uint8_t pinReset) : spi {spi}, pinDataCommand {pinDataCommand}, pinReset {pinReset} {
 }
 
 void St7789::Init() {
-  nrf_gpio_cfg_output(PinMap::LcdDataCommand);
+  nrf_gpio_cfg_output(pinDataCommand);
-  nrf_gpio_cfg_output(PinMap::LcdReset);
+  nrf_gpio_cfg_output(pinReset);
-  nrf_gpio_pin_set(PinMap::LcdReset);
+  nrf_gpio_pin_set(pinReset);
   HardwareReset();
   SoftwareReset();
   SleepOut();
@@ -30,47 +29,77 @@ void St7789::Init() {
   DisplayOn();
 }
 
-void St7789::EnableDataMode(bool isStart) {
-  if (isStart) {
-    nrf_gpio_pin_set(PinMap::LcdDataCommand);
-  }
-}
-
-void St7789::EnableCommandMode(bool isStart) {
-  if (isStart) {
-    nrf_gpio_pin_clear(PinMap::LcdDataCommand);
-  }
-}
-
-void St7789::WriteCommand(uint8_t cmd) {
-  WriteSpi(&cmd, 1, EnableCommandMode);
-}
-
 void St7789::WriteData(uint8_t data) {
-  WriteSpi(&data, 1, EnableDataMode);
+  WriteData(&data, 1);
 }
 
-void St7789::WriteSpi(const uint8_t* data, size_t size, void (*TransactionHook)(bool)) {
+void St7789::WriteData(const uint8_t* data, size_t size) {
-  spi.Write(data, size, TransactionHook);
+  WriteSpi(data, size, [pinDataCommand = pinDataCommand]() {
+    nrf_gpio_pin_set(pinDataCommand);
+  });
+}
+
+void St7789::WriteCommand(uint8_t data) {
+  WriteCommand(&data, 1);
+}
+
+void St7789::WriteCommand(const uint8_t* data, size_t size) {
+  WriteSpi(data, size, [pinDataCommand = pinDataCommand]() {
+    nrf_gpio_pin_clear(pinDataCommand);
+  });
+}
+
+void St7789::WriteSpi(const uint8_t* data, size_t size, const std::function<void()>& preTransactionHook) {
+  spi.Write(data, size, preTransactionHook);
 }
 
 void St7789::SoftwareReset() {
+  EnsureSleepOutPostDelay();
   WriteCommand(static_cast<uint8_t>(Commands::SoftwareReset));
-  nrf_delay_ms(150);
+  // If sleep in: must wait 120ms before sleep out can sent (see driver datasheet)
+  // Unconditionally wait as software reset doesn't need to be performant
+  sleepIn = true;
+  lastSleepExit = xTaskGetTickCount();
+  vTaskDelay(pdMS_TO_TICKS(125));
 }
 
 void St7789::SleepOut() {
+  if (!sleepIn) {
+    return;
+  }
   WriteCommand(static_cast<uint8_t>(Commands::SleepOut));
+  // Wait 5ms for clocks to stabilise
+  // pdMS rounds down => 6 used here
+  vTaskDelay(pdMS_TO_TICKS(6));
+  // Cannot send sleep in or software reset for 120ms
+  lastSleepExit = xTaskGetTickCount();
+  sleepIn = false;
+}
+
+void St7789::EnsureSleepOutPostDelay() {
+  TickType_t delta = xTaskGetTickCount() - lastSleepExit;
+  // Due to timer wraparound, there is a chance of delaying when not necessary
+  // It is very low (pdMS_TO_TICKS(125)/2^32) and waiting an extra 125ms isn't too bad
+  if (delta < pdMS_TO_TICKS(125)) {
+    vTaskDelay(pdMS_TO_TICKS(125) - delta);
+  }
 }
 
 void St7789::SleepIn() {
+  if (sleepIn) {
+    return;
+  }
+  EnsureSleepOutPostDelay();
   WriteCommand(static_cast<uint8_t>(Commands::SleepIn));
+  // Wait 5ms for clocks to stabilise
+  // pdMS rounds down => 6 used here
+  vTaskDelay(pdMS_TO_TICKS(6));
+  sleepIn = true;
 }
 
 void St7789::ColMod() {
   WriteCommand(static_cast<uint8_t>(Commands::ColMod));
   WriteData(0x55);
-  nrf_delay_ms(10);
 }
 
 void St7789::MemoryDataAccessControl() {
@@ -107,12 +136,10 @@ void St7789::RowAddressSet() {
 
 void St7789::DisplayInversionOn() {
   WriteCommand(static_cast<uint8_t>(Commands::DisplayInversionOn));
-  nrf_delay_ms(10);
 }
 
 void St7789::NormalModeOn() {
   WriteCommand(static_cast<uint8_t>(Commands::NormalModeOn));
-  nrf_delay_ms(10);
 }
 
 void St7789::DisplayOn() {
@@ -131,12 +158,11 @@ void St7789::SetAddrWindow(uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1) {
   WriteData(y0 & 0xff);
   WriteData(y1 >> 8);
   WriteData(y1 & 0xff);
-
-  WriteToRam();
 }
 
-void St7789::WriteToRam() {
+void St7789::WriteToRam(const uint8_t* data, size_t size) {
   WriteCommand(static_cast<uint8_t>(Commands::WriteToRam));
+  WriteData(data, size);
 }
 
 void St7789::SetVdv() {
@@ -148,7 +174,6 @@ void St7789::SetVdv() {
 
 void St7789::DisplayOff() {
   WriteCommand(static_cast<uint8_t>(Commands::DisplayOff));
-  nrf_delay_ms(500);
 }
 
 void St7789::VerticalScrollStartAddress(uint16_t line) {
@@ -163,23 +188,28 @@ void St7789::Uninit() {
 
 void St7789::DrawBuffer(uint16_t x, uint16_t y, uint16_t width, uint16_t height, const uint8_t* data, size_t size) {
   SetAddrWindow(x, y, x + width - 1, y + height - 1);
-  WriteSpi(data, size, EnableDataMode);
+  WriteToRam(data, size);
 }
 
 void St7789::HardwareReset() {
-  nrf_gpio_pin_clear(PinMap::LcdReset);
+  nrf_gpio_pin_clear(pinReset);
-  nrf_delay_ms(10);
+  vTaskDelay(pdMS_TO_TICKS(1));
-  nrf_gpio_pin_set(PinMap::LcdReset);
+  nrf_gpio_pin_set(pinReset);
+  // If hardware reset started while sleep out, reset time may be up to 120ms
+  // Unconditionally wait as hardware reset doesn't need to be performant
+  sleepIn = true;
+  lastSleepExit = xTaskGetTickCount();
+  vTaskDelay(pdMS_TO_TICKS(125));
 }
 
 void St7789::Sleep() {
   SleepIn();
-  nrf_gpio_cfg_default(PinMap::LcdDataCommand);
+  nrf_gpio_cfg_default(pinDataCommand);
   NRF_LOG_INFO("[LCD] Sleep");
 }
 
 void St7789::Wakeup() {
-  nrf_gpio_cfg_output(PinMap::LcdDataCommand);
+  nrf_gpio_cfg_output(pinDataCommand);
   SleepOut();
   VerticalScrollStartAddress(verticalScrollingStartAddress);
   DisplayOn();

src/drivers/St7789.h

@@ -1,6 +1,9 @@
 #pragma once
 #include <cstddef>
 #include <cstdint>
+#include <functional>
+
+#include <FreeRTOS.h>
 
 namespace Pinetime {
   namespace Drivers {
@@ -8,7 +11,7 @@ namespace Pinetime {
 
     class St7789 {
     public:
-      explicit St7789(Spi& spi);
+      explicit St7789(Spi& spi, uint8_t pinDataCommand, uint8_t pinReset);
       St7789(const St7789&) = delete;
       St7789& operator=(const St7789&) = delete;
       St7789(St7789&&) = delete;
@@ -26,26 +29,30 @@ namespace Pinetime {
 
     private:
       Spi& spi;
+      uint8_t pinDataCommand;
+      uint8_t pinReset;
       uint8_t verticalScrollingStartAddress = 0;
+      bool sleepIn;
+      TickType_t lastSleepExit;
 
       void HardwareReset();
       void SoftwareReset();
       void SleepOut();
+      void EnsureSleepOutPostDelay();
       void SleepIn();
       void ColMod();
       void MemoryDataAccessControl();
       void DisplayInversionOn();
       void NormalModeOn();
-      void WriteToRam();
+      void WriteToRam(const uint8_t* data, size_t size);
       void DisplayOn();
       void DisplayOff();
 
       void SetAddrWindow(uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1);
       void SetVdv();
       void WriteCommand(uint8_t cmd);
-      void WriteSpi(const uint8_t* data, size_t size, void (*TransactionHook)(bool));
+      void WriteCommand(const uint8_t* data, size_t size);
-      static void EnableDataMode(bool isStart);
+      void WriteSpi(const uint8_t* data, size_t size, const std::function<void()>& preTransactionHook);
-      static void EnableCommandMode(bool isStart);
 
       enum class Commands : uint8_t {
         SoftwareReset = 0x01,
@@ -65,6 +72,7 @@ namespace Pinetime {
         VdvSet = 0xc4,
       };
       void WriteData(uint8_t data);
+      void WriteData(const uint8_t* data, size_t size);
       void ColumnAddressSet();
 
       static constexpr uint16_t Width = 240;

src/main.cpp

@@ -68,7 +68,7 @@ Pinetime::Drivers::SpiMaster spi {Pinetime::Drivers::SpiMaster::SpiModule::SPI0,
                                    Pinetime::PinMap::SpiMiso}};
 
 Pinetime::Drivers::Spi lcdSpi {spi, Pinetime::PinMap::SpiLcdCsn};
-Pinetime::Drivers::St7789 lcd {lcdSpi};
+Pinetime::Drivers::St7789 lcd {lcdSpi, Pinetime::PinMap::LcdDataCommand, Pinetime::PinMap::LcdReset};
 
 Pinetime::Drivers::Spi flashSpi {spi, Pinetime::PinMap::SpiFlashCsn};
 Pinetime::Drivers::SpiNorFlash spiNorFlash {flashSpi};

src/recoveryLoader.cpp

@@ -45,7 +45,7 @@ Pinetime::Drivers::Spi flashSpi {spi, Pinetime::PinMap::SpiFlashCsn};
 Pinetime::Drivers::SpiNorFlash spiNorFlash {flashSpi};
 
 Pinetime::Drivers::Spi lcdSpi {spi, Pinetime::PinMap::SpiLcdCsn};
-Pinetime::Drivers::St7789 lcd {lcdSpi};
+Pinetime::Drivers::St7789 lcd {lcdSpi, Pinetime::PinMap::LcdDataCommand, Pinetime::PinMap::LcdReset};
 
 Pinetime::Controllers::BrightnessController brightnessController;
 
@@ -121,7 +121,6 @@ void DisplayLogo() {
   Pinetime::Tools::RleDecoder rleDecoder(infinitime_nb, sizeof(infinitime_nb));
   for (int i = 0; i < displayWidth; i++) {
     rleDecoder.DecodeNext(displayBuffer, displayWidth * bytesPerPixel);
-    ulTaskNotifyTake(pdTRUE, 500);
     lcd.DrawBuffer(0, i, displayWidth, 1, reinterpret_cast<const uint8_t*>(displayBuffer), displayWidth * bytesPerPixel);
   }
 }
@@ -130,7 +129,6 @@ void DisplayProgressBar(uint8_t percent, uint16_t color) {
   static constexpr uint8_t barHeight = 20;
   std::fill(displayBuffer, displayBuffer + (displayWidth * bytesPerPixel), color);
   for (int i = 0; i < barHeight; i++) {
-    ulTaskNotifyTake(pdTRUE, 500);
     uint16_t barWidth = std::min(static_cast<float>(percent) * 2.4f, static_cast<float>(displayWidth));
     lcd.DrawBuffer(0, displayWidth - barHeight + i, barWidth, 1, reinterpret_cast<const uint8_t*>(displayBuffer), barWidth * bytesPerPixel);
   }