Decouple PicoDVI class allowing use as base for more framebuffer types

Readme.md

@@ -3,7 +3,7 @@ Special Fork of PicoDVI for RP2040/RP2350 Arduino Philhower Core + Adafruit_GFX
 
 At the moment this implements a 16-bit color framebuffer to which Adafruit_GFX
 drawing operations can be made. Other framebuffer types (8-bit, 1-bit) might
-get added later, but not every permutation PicoDVI is capable of.
+get added later, but probably not every permutation PicoDVI is capable of.
 
 Requires Earle Philhower III RP2040 Arduino core (not the "official" Arduino
 RP2040 core). Oddly, some resolutions may require slightly higher over-voltage
@@ -28,10 +28,8 @@ like it for both Pico SDK and Arduino IDE use from a single repo.
 
 Roadmap:
 - Other GFX-compatible framebuffer depths & sizes.
-- Might split out the PicoDVI-to-Arduino wrapper elements from the GFX parts
-so lower-level access (scanline callbacks, etc.) can be done in Arduino, to
-handle any other display permutations and perhaps port over some of PicoDVI's
-Pico SDK examples (more soft-links, e.g. libsprite, might be needed).
+- Perhaps some non-GFX modes to bring over some of the Pico SDK examples in
+an Arduino context.
 
 Bitbanged DVI on the RP2040 Microcontroller
 ===========================================

examples/hello/hello.ino

@@ -3,15 +3,17 @@
 
 #include <PicoDVI.h>
 
-PicoDVI display(320, 240, VREG_VOLTAGE_1_20, dvi_timing_640x480p_60hz, pimoroni_demo_hdmi_cfg);
+DVIGFX16 display(320, 240, dvi_timing_640x480p_60hz, VREG_VOLTAGE_1_20, pimoroni_demo_hdmi_cfg);
 // Not all RP2040s can deal with the 295 MHz overclock this requires, but if you'd like to try:
-//PicoDVI display(400, 240, VREG_VOLTAGE_1_30, dvi_timing_800x480p_60hz, pimoroni_demo_hdmi_cfg);
+//DVIGFX16 display(400, 240, dvi_timing_800x480p_60hz, VREG_VOLTAGE_1_30, pimoroni_demo_hdmi_cfg);
 
 void setup() {
   Serial.begin(115200);
   //while(!Serial);
-  bool status = display.begin();
-  Serial.println(status);
+  if (!display.begin()) {
+    pinMode(LED_BUILTIN, OUTPUT);
+    for (;;) digitalWrite(LED_BUILTIN, (millis() / 500) & 1);
+  }
 }
 
 #if 0

src/PicoDVI.cpp

@@ -1,64 +1,83 @@
 #include "PicoDVI.h"
 
-// Some elements of the PicoDVI object must be accessed in interrupts or
-// outside the class context, so a pointer to the active PicoDVI object is
-// kept. This does mean only one instance can be active, but that's the
-// typical use case anyway so we should be OK.
-static PicoDVI *dviptr = NULL;
+// PicoDVI class encapsulates some of the libdvi functionality -------------
+// Subclasses then implement specific display types.
+
+static PicoDVI *dviptr = NULL; // For C access to active C++ object
 static volatile bool wait_begin = true;
 
-// This runs at startup on core 1
-void setup1() {
+// Runs on core 1 on startup
+void setup1(void) {
   while (wait_begin)
-    ; // Wait for PicoDVI::begin() to do its thing
+    ; // Wait for DVIGFX*::begin() to do its thing on core 0
   dviptr->_setup();
 }
 
+// Runs on core 1 after wait_begin released
 void PicoDVI::_setup(void) {
   dvi_register_irqs_this_core(&dvi0, DMA_IRQ_0);
   dvi_start(&dvi0);
-  dvi_scanbuf_main_16bpp(&dvi0);
+  (*mainloop)(&dvi0);
 }
 
-static void core1_scanline_callback(void) { dviptr->_scanline_callback(); }
+PicoDVI::PicoDVI(const struct dvi_timing &t, vreg_voltage v,
+         const struct dvi_serialiser_cfg &c)
+    : voltage(v) {
+  dvi0.timing = &t;
+  memcpy(&dvi0.ser_cfg, &c, sizeof dvi0.ser_cfg);
+};
 
-void PicoDVI::_scanline_callback(void) {
+PicoDVI::~PicoDVI(void) {
+  dviptr = NULL;
+  wait_begin = true;
+}
+
+void PicoDVI::begin(void) {
+  dviptr = this;
+  vreg_set_voltage(voltage);
+  delay(10);
+  set_sys_clock_khz(dvi0.timing->bit_clk_khz, true); // Run at TMDS bit clock
+  dvi_init(&dvi0, next_striped_spin_lock_num(), next_striped_spin_lock_num());
+}
+
+// DVIGFX16 class provides GFX-compatible 16-bit color framebuffer ---------
+
+static void *gfxptr = NULL; // For C access to active C++ object
+
+DVIGFX16::DVIGFX16(const uint16_t w, const uint16_t h,
+                   const struct dvi_timing &t, vreg_voltage v,
+                   const struct dvi_serialiser_cfg &c)
+    : PicoDVI(t, v, c), GFXcanvas16(w, h) {}
+
+static void scanline_callback_GFX16(void) {
+  ((DVIGFX16 *)gfxptr)->_scanline_callback();
+}
+
+DVIGFX16::~DVIGFX16(void) {
+  gfxptr = NULL;
+}
+
+void DVIGFX16::_scanline_callback(void) {
   // Discard any scanline pointers passed back
   uint16_t *bufptr;
   while (queue_try_remove_u32(&dvi0.q_colour_free, &bufptr))
     ;
-  // Note first two scanlines are pushed before DVI start
-  static uint scanline = 2;
   bufptr = &getBuffer()[WIDTH * scanline];
   queue_add_blocking_u32(&dvi0.q_colour_valid, &bufptr);
   scanline = (scanline + 1) % HEIGHT;
 }
 
-PicoDVI::PicoDVI(uint16_t w, uint16_t h, vreg_voltage v,
-                 const struct dvi_timing &t, const struct dvi_serialiser_cfg &c)
-    : GFXcanvas16(w, h), timing(&t), voltage(v), cfg(&c) {}
-
-PicoDVI::~PicoDVI(void) { dviptr = NULL; }
-
-bool PicoDVI::begin(void) {
-  if (getBuffer()) { // Canvas alloc'd OK?
-    vreg_set_voltage(voltage);
-    delay(10);
-    set_sys_clock_khz(timing->bit_clk_khz, true); // Run at TMDS bit clock
-
-    dvi0.timing = timing;
-    memcpy(&dvi0.ser_cfg, cfg, sizeof dvi0.ser_cfg);
-    dvi0.scanline_callback = core1_scanline_callback;
-    dvi_init(&dvi0, next_striped_spin_lock_num(), next_striped_spin_lock_num());
-
-    uint16_t *bufptr = getBuffer();
+bool DVIGFX16::begin(void) {
+  uint16_t *bufptr = getBuffer();
+  if ((bufptr)) {
+    gfxptr = this;
+    mainloop = dvi_scanbuf_main_16bpp; // in libdvi
+    dvi0.scanline_callback = scanline_callback_GFX16;
+    PicoDVI::begin();
     queue_add_blocking_u32(&dvi0.q_colour_valid, &bufptr);
     bufptr += WIDTH;
     queue_add_blocking_u32(&dvi0.q_colour_valid, &bufptr);
-
-    dviptr = this;
-    wait_begin = false; // Set core 1 free
-
+    wait_begin = false; // Set core 1 in motion
     return true;
   }
   return false;

src/PicoDVI.h

@@ -6,24 +6,34 @@
 #include "pico/stdlib.h" // In Pico SDK
 #include <Adafruit_GFX.h>
 
-class PicoDVI : public GFXcanvas16 {
+class PicoDVI {
 public:
-  PicoDVI(uint16_t w, uint16_t h, vreg_voltage v, const struct dvi_timing &t,
-          const struct dvi_serialiser_cfg &c);
+  PicoDVI(const struct dvi_timing &t = dvi_timing_640x480p_60hz,
+      vreg_voltage v = VREG_VOLTAGE_1_10,
+      const struct dvi_serialiser_cfg &c = pimoroni_demo_hdmi_cfg);
   ~PicoDVI(void);
+  void _setup(void);
+
+protected:
+  void begin(void);
+  vreg_voltage voltage;
+  struct dvi_inst dvi0;
+  void (*mainloop)(dvi_inst *) = NULL;
+};
+
+class DVIGFX16 : public PicoDVI, public GFXcanvas16 {
+public:
+  DVIGFX16(const uint16_t w = 320, const uint16_t h = 240,
+           const struct dvi_timing &t = dvi_timing_640x480p_60hz,
+           vreg_voltage v = VREG_VOLTAGE_1_10,
+           const struct dvi_serialiser_cfg &c = pimoroni_demo_hdmi_cfg);
+  ~DVIGFX16(void);
   bool begin(void);
   uint16_t color565(uint8_t red, uint8_t green, uint8_t blue) {
     return ((red & 0xF8) << 8) | ((green & 0xFC) << 3) | (blue >> 3);
   }
-
-  // These functions are necessary to lib but can't be private/protected.
-  // DO NOT CALL these functions in user code! Pretend they're not here.
-  void _setup(void);
   void _scanline_callback(void);
 
 protected:
-  struct dvi_inst dvi0;
-  const struct dvi_timing *timing;
-  vreg_voltage voltage;
-  const struct dvi_serialiser_cfg *cfg;
+  uint16_t scanline = 2; // First 2 scanlines are set up before DVI start
 };