o Move the concept of a pin-pulser to a class, so that we can create

  different implementations.

include/gpio.h

@@ -18,17 +18,11 @@
 
 #include <stdint.h>
 
+#include <vector>
+
 // Putting this in our namespace to not collide with other things called like
 // this.
 namespace rgb_matrix {
-
-// More precise than default OS provided timing.
-class Timers {
-public:
-  static bool Init();
-  static void sleep_nanos(long t);
-};
-
 // For now, everything is initialized as output.
 class GPIO {
  public:
@@ -65,5 +59,25 @@ class GPIO {
   uint32_t output_bits_;
   volatile uint32_t *gpio_port_;
 };
+
+// A PinPulser is a utility class that pulses a GPIO pin. There can be various
+// implementations.
+class PinPulser {
+public:
+  // Factory for a PinPulser. Chooses the right implementation depending
+  // on the context (CPU and which pins are affected).
+  // "gpio_mask" is the mask that should be output (since we only
+  //   need negative pulses, this is what it does)
+  // "nano_wait_spec" contains a list of time periods we'd like
+  //   invoke later. This can be used to pre-process timings if needed.
+  static PinPulser *Create(GPIO *io, uint32_t gpio_mask,
+                           const std::vector<int> &nano_wait_spec);
+
+  virtual ~PinPulser() {}
+
+  // Send a pulse with a given length (index into nano_wait_spec array).
+  virtual void SendPulse(int time_spec_number) = 0;
+};
+
 }  // end namespace rgb_matrix
 #endif  // RPI_GPIO_H

lib/framebuffer-internal.h

@@ -19,6 +19,7 @@
 
 namespace rgb_matrix {
 class GPIO;
+class PinPulser;
 namespace internal {
 // Internal representation of the frame-buffer that as well can
 // write itself to GPIO.
@@ -29,7 +30,7 @@ class Framebuffer {
   Framebuffer(int rows, int columns, int parallel);
   ~Framebuffer();
 
-  // Initialize GPIO bits for output.
+  // Initialize GPIO bits for output. Only call once.
   void InitGPIO(GPIO *io);
 
   // Set PWM bits used for output. Default is 11, but if you only deal with
@@ -67,6 +68,8 @@ class Framebuffer {
   const int double_rows_;
   const uint8_t row_mask_;
 
+  PinPulser *output_enable_pulser_;
+
 #ifdef ADAFRUIT_RGBMATRIX_HAT
   // Adafruit made a HAT to work with this library, but it has a slightly
   // different GPIO mapping. This is this mapping. See #else for regular mapping.

lib/framebuffer.cc

@@ -53,7 +53,8 @@ Framebuffer::Framebuffer(int rows, int columns, int parallel)
   : rows_(rows), parallel_(parallel), height_(rows * parallel),
     columns_(columns),
     pwm_bits_(kBitPlanes), do_luminance_correct_(true),
-    double_rows_(rows / 2), row_mask_(double_rows_ - 1) {
+    double_rows_(rows / 2), row_mask_(double_rows_ - 1),
+    output_enable_pulser_(NULL) {
   bitplane_buffer_ = new IoBits [double_rows_ * columns_ * kBitPlanes];
   Clear();
   assert(rows_ <= 32);
@@ -69,6 +70,7 @@ Framebuffer::Framebuffer(int rows, int columns, int parallel)
 
 Framebuffer::~Framebuffer() {
   delete [] bitplane_buffer_;
+  delete output_enable_pulser_;
 }
 
 /* static */ void Framebuffer::InitGPIO(GPIO *io) {
@@ -105,6 +107,21 @@ Framebuffer::~Framebuffer() {
   // Initialize outputs, make sure that all of these are supported bits.
   const uint32_t result = io->InitOutputs(b.raw);
   assert(result == b.raw);
+
+  // Now, set up the PinPulser for output enable.
+  IoBits output_enable_bits;
+#ifdef SUPPORT_CLASSIC_LED_GPIO_WIRING_
+  output_enable_bits.bits.output_enable_rev1
+    = output_enable_bits.bits.output_enable_rev2 = 1;
+#endif
+  output_enable_bits.bits.output_enable = 1;
+
+  std::vector<int> bitplane_timings;
+  for (int b = 0; b < kBitPlanes; ++b) {
+    bitplane_timings.push_back(kBaseTimeNanos << b);
+  }
+  output_enable_pulser_ = PinPulser::Create(io, output_enable_bits.raw,
+                                            bitplane_timings);
 }
 
 bool Framebuffer::SetPWMBits(uint8_t value) {
@@ -306,14 +323,11 @@ void Framebuffer::DumpToMatrix(GPIO *io) {
   IoBits row_mask;
   row_mask.bits.a = row_mask.bits.b = row_mask.bits.c = row_mask.bits.d = 1;
 
-  IoBits clock, output_enable, strobe, row_address;
+  IoBits clock, strobe, row_address;
 #ifdef SUPPORT_CLASSIC_LED_GPIO_WIRING_
   clock.bits.clock_rev1 = clock.bits.clock_rev2 = 1;
-  output_enable.bits.output_enable_rev1
-    = output_enable.bits.output_enable_rev2 = 1;
 #endif
   clock.bits.clock = 1;
-  output_enable.bits.output_enable = 1;
   strobe.bits.strobe = 1;
 
   const int pwm_to_show = pwm_bits_;  // Local copy, might change in process.
@@ -342,9 +356,7 @@ void Framebuffer::DumpToMatrix(GPIO *io) {
       io->ClearBits(strobe.raw);
 
       // Now switch on for the sleep time necessary for that bit-plane.
-      io->ClearBits(output_enable.raw);
-      Timers::sleep_nanos(kBaseTimeNanos << b);
-      io->SetBits(output_enable.raw);
+      output_enable_pulser_->SendPulse(b);
     }
   }
 }

lib/gpio.cc

@@ -120,9 +120,7 @@ bool GPIO::Init() {
   if (gpio_port_ == NULL) {
     return false;
   }
-
-  // To be compatible with old code, make sure to initialize this here.
-  return Timers::Init();
+  return true;
 }
 
 void GPIO::SetBits(uint32_t value) {
@@ -139,6 +137,35 @@ void GPIO::ClearBits(uint32_t value) {
 #endif
 }
 
+// --- PinPulser. Private implementation parts.
+namespace {
+// Manual timers.
+class Timers {
+public:
+  static bool Init();
+  static void sleep_nanos(long t);
+};
+
+// Simplest of PinPulsers. Uses somewhat jittery and manual timers
+// to get the timing, but not optimal.
+class TimerBasedPinPulser : public PinPulser {
+public:
+  TimerBasedPinPulser(GPIO *io, uint32_t bits,
+                      const std::vector<int> &nano_specs)
+    : io_(io), bits_(bits), nano_specs_(nano_specs) {}
+
+  virtual void SendPulse(int time_spec_number) {
+    io_->ClearBits(bits_);
+    Timers::sleep_nanos(nano_specs_[time_spec_number]);
+    io_->SetBits(bits_);
+  }
+
+private:
+  GPIO *const io_;
+  const uint32_t bits_;
+  const std::vector<int> nano_specs_;
+};
+
 // ----------
 // TODO: timing needs to be improved. It is jittery due to the nature of running
 // in a non-realtime operating system, and apparently the nanosleep() does not
@@ -207,5 +234,14 @@ static void sleep_nanos_rpi_2(long nanos) {
     asm("");
   }
 }
+} // end anonymous namespace
+
+// Public PinPulser factory
+PinPulser *PinPulser::Create(GPIO *io, uint32_t gpio_mask,
+                             const std::vector<int> &nano_wait_spec) {
+  // The only implementation so far.
+  if (!Timers::Init()) return NULL;
+  return new TimerBasedPinPulser(io, gpio_mask, nano_wait_spec);
+}
 
 } // namespace rgb_matrix