dev, arm: Rewrite the HDLCD controller

Rewrite the HDLCD controller to use the new DMA engine and pixel
pump. This fixes several bugs in the current implementation:

   * Broken/missing interrupt support (VSync, underrun, DMA end)
   * Fragile resolution changes (changing resolutions used
     to cause assertion errors).
   * Support for resolutions with a width that isn't divisible by 32.
   * The pixel clock can now be set dynamically.

This breaks checkpoint compatibility. Checkpoints can be upgraded with
the checkpoint conversion script. However, upgraded checkpoints won't
contain the state of the current frame. That means that HDLCD
controllers restoring from a converted checkpoint immediately start
drawing a new frame (i.e, expect timing differences).

3 files changed, 609 insertions, 919 deletions

diff --git a/src/dev/arm/RealView.py b/src/dev/arm/RealView.py
index a9b9dede2..dc8868b9f 100644
--- a/src/dev/arm/RealView.py
+++ b/src/dev/arm/RealView.py
@@ -54,6 +54,7 @@ from Uart import Uart
 from SimpleMemory import SimpleMemory
 from Gic import *
 from EnergyCtrl import EnergyCtrl
+from ClockDomain import SrcClockDomain
 
 class AmbaPioDevice(BasicPioDevice):
     type = 'AmbaPioDevice'
@@ -218,23 +219,23 @@ class Pl111(AmbaDmaDevice):
     amba_id = 0x00141111
     enable_capture = Param.Bool(True, "capture frame to system.framebuffer.bmp")
 
-
 class HDLcd(AmbaDmaDevice):
     type = 'HDLcd'
     cxx_header = "dev/arm/hdlcd.hh"
-    # For reference, 1024x768MR-16@60  ~= 56 MHz
-    #                1920x1080MR-16@60 ~= 137 MHz
-    #                3840x2160MR-16@60 ~= 533 MHz
-    # Match against the resolution selected in the Linux DTS/DTB file.
-    pixel_clock = Param.Clock('137MHz', "Clock frequency of the pixel clock "
-                                        "(i.e. PXLREFCLK / OSCCLK 5")
     vnc = Param.VncInput(Parent.any, "Vnc server for remote frame buffer "
                                      "display")
     amba_id = 0x00141000
     workaround_swap_rb = Param.Bool(True, "Workaround incorrect color "
                                     "selector order in some kernels")
+    workaround_dma_line_count = Param.Bool(True, "Workaround incorrect "
+                                           "DMA line count (off by 1)")
     enable_capture = Param.Bool(True, "capture frame to system.framebuffer.bmp")
 
+    pixel_buffer_size = Param.MemorySize32("2kB", "Size of address range")
+
+    pxl_clk = Param.ClockDomain("Pixel clock source")
+    pixel_chunk = Param.Unsigned(32, "Number of pixels to handle in one batch")
+
 class RealView(Platform):
     type = 'RealView'
     cxx_header = "dev/arm/realview.hh"
@@ -518,7 +519,8 @@ class VExpress_EMM(RealView):
     timer0 = Sp804(int_num0=34, int_num1=34, pio_addr=0x1C110000, clock0='1MHz', clock1='1MHz')
     timer1 = Sp804(int_num0=35, int_num1=35, pio_addr=0x1C120000, clock0='1MHz', clock1='1MHz')
     clcd   = Pl111(pio_addr=0x1c1f0000, int_num=46)
-    hdlcd  = HDLcd(pio_addr=0x2b000000, int_num=117)
+    hdlcd  = HDLcd(pxl_clk=realview_io.osc_pxl,
+                   pio_addr=0x2b000000, int_num=117)
     kmi0   = Pl050(pio_addr=0x1c060000, int_num=44)
     kmi1   = Pl050(pio_addr=0x1c070000, int_num=45, is_mouse=True)
     vgic   = VGic(vcpu_addr=0x2c006000, hv_addr=0x2c004000, ppint=25)
diff --git a/src/dev/arm/hdlcd.cc b/src/dev/arm/hdlcd.cc
index afeef7637..84df24472 100644
--- a/src/dev/arm/hdlcd.cc
+++ b/src/dev/arm/hdlcd.cc
@@ -35,6 +35,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  * Authors: Chris Emmons
+ *          Andreas Sandberg
  */
 
 #include "dev/arm/hdlcd.hh"
@@ -42,145 +43,175 @@
 #include "base/vnc/vncinput.hh"
 #include "base/output.hh"
 #include "base/trace.hh"
+#include "debug/Checkpoint.hh"
 #include "debug/HDLcd.hh"
-#include "debug/Uart.hh"
 #include "dev/arm/amba_device.hh"
 #include "dev/arm/base_gic.hh"
 #include "mem/packet.hh"
 #include "mem/packet_access.hh"
+#include "params/HDLcd.hh"
 #include "sim/system.hh"
 
 using std::vector;
 
 
 // initialize hdlcd registers
-HDLcd::HDLcd(const Params *p)
-    : AmbaDmaDevice(p), version(VERSION_RESETV),
-      int_rawstat(0), int_clear(0), int_mask(0), int_status(0),
+HDLcd::HDLcd(const HDLcdParams *p)
+    : AmbaDmaDevice(p, 0xFFFF),
+      // Parameters
+      vnc(p->vnc),
+      workaroundSwapRB(p->workaround_swap_rb),
+      workaroundDmaLineCount(p->workaround_dma_line_count),
+      addrRanges{RangeSize(pioAddr, pioSize)},
+      enableCapture(p->enable_capture),
+      pixelBufferSize(p->pixel_buffer_size),
+
+      // Registers
+      version(VERSION_RESETV),
+      int_rawstat(0), int_mask(0),
+
       fb_base(0), fb_line_length(0), fb_line_count(0), fb_line_pitch(0),
       bus_options(BUS_OPTIONS_RESETV),
+
       v_sync(0), v_back_porch(0), v_data(0), v_front_porch(0),
       h_sync(0), h_back_porch(0), h_data(0), h_front_porch(0),
-      polarities(0), command(0), pixel_format(0),
-      red_select(0), green_select(0), blue_select(0),
-      pixelClock(p->pixel_clock),
-      fb(0, 0), vnc(p->vnc), bmp(&fb), pic(NULL),
-      frameReadStartTime(0),
-      dmaStartAddr(0), dmaCurAddr(0), dmaMaxAddr(0), dmaPendingNum(0),
-      frameUnderrun(false), pixelBufferSize(0),
-      pixelIndex(0), doUpdateParams(false), frameUnderway(false),
-      dmaBytesInFlight(0),
-      startFrameEvent(this), endFrameEvent(this), renderPixelEvent(this),
-      fillPixelBufferEvent(this), intEvent(this),
-      dmaDoneEventAll(MAX_OUTSTANDING_DMA_REQ_CAPACITY, this),
-      dmaDoneEventFree(MAX_OUTSTANDING_DMA_REQ_CAPACITY),
-      enableCapture(p->enable_capture),
-      workaround_swap_rb(p->workaround_swap_rb)
-{
-    pioSize = 0xFFFF;
+      polarities(0),
 
-    for (int i = 0; i < MAX_OUTSTANDING_DMA_REQ_CAPACITY; ++i)
-        dmaDoneEventFree[i] = &dmaDoneEventAll[i];
+      command(0),
+
+      pixel_format(0),
+      red_select(0), green_select(0), blue_select(0),
 
+      // Other
+      bmp(&pixelPump.fb), pic(NULL), conv(PixelConverter::rgba8888_le),
+      pixelPump(*this, *p->pxl_clk, p->pixel_chunk)
+{
     if (vnc)
-        vnc->setFrameBuffer(&fb);
+        vnc->setFrameBuffer(&pixelPump.fb);
 }
 
 HDLcd::~HDLcd()
 {
 }
 
+void
+HDLcd::serialize(CheckpointOut &cp) const
+{
+    DPRINTF(Checkpoint, "Serializing ARM HDLCD\n");
+
+    SERIALIZE_SCALAR(int_rawstat);
+    SERIALIZE_SCALAR(int_mask);
+
+    SERIALIZE_SCALAR(fb_base);
+    SERIALIZE_SCALAR(fb_line_length);
+    SERIALIZE_SCALAR(fb_line_count);
+    SERIALIZE_SCALAR(fb_line_pitch);
+    SERIALIZE_SCALAR(bus_options);
+
+    SERIALIZE_SCALAR(v_sync);
+    SERIALIZE_SCALAR(v_back_porch);
+    SERIALIZE_SCALAR(v_data);
+    SERIALIZE_SCALAR(v_front_porch);
+
+    SERIALIZE_SCALAR(h_sync);
+    SERIALIZE_SCALAR(h_back_porch);
+    SERIALIZE_SCALAR(h_data);
+    SERIALIZE_SCALAR(h_front_porch);
+
+    SERIALIZE_SCALAR(polarities);
+
+    SERIALIZE_SCALAR(command);
+    SERIALIZE_SCALAR(pixel_format);
+    SERIALIZE_SCALAR(red_select);
+    SERIALIZE_SCALAR(green_select);
+    SERIALIZE_SCALAR(blue_select);
+
+    SERIALIZE_OBJ(pixelPump);
+    if (enabled())
+        dmaEngine->serializeSection(cp, "dmaEngine");
+}
+
+void
+HDLcd::unserialize(CheckpointIn &cp)
+{
+    DPRINTF(Checkpoint, "Unserializing ARM HDLCD\n");
+
+    UNSERIALIZE_SCALAR(int_rawstat);
+    UNSERIALIZE_SCALAR(int_mask);
+
+    UNSERIALIZE_SCALAR(fb_base);
+    UNSERIALIZE_SCALAR(fb_line_length);
+    UNSERIALIZE_SCALAR(fb_line_count);
+    UNSERIALIZE_SCALAR(fb_line_pitch);
+    UNSERIALIZE_SCALAR(bus_options);
+
+    UNSERIALIZE_SCALAR(v_sync);
+    UNSERIALIZE_SCALAR(v_back_porch);
+    UNSERIALIZE_SCALAR(v_data);
+    UNSERIALIZE_SCALAR(v_front_porch);
+
+    UNSERIALIZE_SCALAR(h_sync);
+    UNSERIALIZE_SCALAR(h_back_porch);
+    UNSERIALIZE_SCALAR(h_data);
+    UNSERIALIZE_SCALAR(h_front_porch);
+
+    UNSERIALIZE_SCALAR(polarities);
+
+    UNSERIALIZE_SCALAR(command);
+    UNSERIALIZE_SCALAR(pixel_format);
+    UNSERIALIZE_SCALAR(red_select);
+    UNSERIALIZE_SCALAR(green_select);
+    UNSERIALIZE_SCALAR(blue_select);
+
+    {
+        // Try to unserialize the pixel pump. It might not exist if
+        // we're unserializing an old checkpoint.
+        ScopedCheckpointSection sec(cp, "pixelPump");
+        if (cp.sectionExists(Serializable::currentSection()))
+            pixelPump.unserialize(cp);
+    }
+
+    if (enabled()) {
+        // Create the DMA engine and read its state from the
+        // checkpoint. We don't need to worry about the pixel pump as
+        // it is a proper SimObject.
+        createDmaEngine();
+        dmaEngine->unserializeSection(cp, "dmaEngine");
+
+        conv = pixelConverter();
+    }
+}
+
+void
+HDLcd::drainResume()
+{
+    AmbaDmaDevice::drainResume();
+
+    // We restored from an old checkpoint without a pixel pump, start
+    // an new refresh. This typically happens when restoring from old
+    // checkpoints.
+    if (enabled() && !pixelPump.active())
+        pixelPump.start(displayTimings());
+
+    // We restored from a checkpoint and need to update the VNC server
+    if (pixelPump.active() && vnc)
+        vnc->setDirty();
+}
+
 // read registers and frame buffer
 Tick
 HDLcd::read(PacketPtr pkt)
 {
-    uint32_t data = 0;
-    const Addr daddr = pkt->getAddr() - pioAddr;
-
-    DPRINTF(HDLcd, "read register BASE+0x%04x size=%d\n", daddr,
-            pkt->getSize());
-
     assert(pkt->getAddr() >= pioAddr &&
-            pkt->getAddr() < pioAddr + pioSize &&
-            pkt->getSize() == 4);
+           pkt->getAddr() < pioAddr + pioSize);
 
-    switch (daddr) {
-      case Version:
-        data = version;
-        break;
-      case Int_RawStat:
-        data = int_rawstat;
-        break;
-      case Int_Clear:
-        panic("HDLCD INT_CLEAR register is Write-Only\n");
-        break;
-      case Int_Mask:
-        data = int_mask;
-        break;
-      case Int_Status:
-        data = int_status;
-        break;
-      case Fb_Base:
-        data = fb_base;
-        break;
-      case Fb_Line_Length:
-        data = fb_line_length;
-        break;
-      case Fb_Line_Count:
-        data = fb_line_count;
-        break;
-      case Fb_Line_Pitch:
-        data = fb_line_pitch;
-        break;
-      case Bus_Options:
-        data = bus_options;
-        break;
-      case V_Sync:
-        data = v_sync;
-        break;
-      case V_Back_Porch:
-        data = v_back_porch;
-        break;
-      case V_Data:
-        data = v_data;
-        break;
-      case V_Front_Porch:
-        data = v_front_porch;
-        break;
-      case H_Sync:
-        data = h_sync;
-        break;
-      case H_Back_Porch:
-        data = h_back_porch;
-        break;
-      case H_Data:
-        data = h_data;
-        break;
-      case H_Front_Porch:
-        data = h_front_porch;
-        break;
-      case Polarities:
-        data = polarities;
-        break;
-      case Command:
-        data = command;
-        break;
-      case Pixel_Format:
-        data = pixel_format;
-        break;
-      case Red_Select:
-        data = red_select;
-        break;
-      case Green_Select:
-        data = green_select;
-        break;
-      case Blue_Select:
-        data = blue_select;
-        break;
-      default:
-        panic("Tried to read HDLCD register that doesn't  exist\n", daddr);
-        break;
-    }
+    const Addr daddr(pkt->getAddr() - pioAddr);
+    panic_if(pkt->getSize() != 4,
+             "Unhandled read size (address: 0x.4x, size: %u)",
+             daddr, pkt->getSize());
+
+    const uint32_t data(readReg(daddr));
+    DPRINTF(HDLcd, "read register 0x%04x: 0x%x\n", daddr, data);
 
     pkt->set<uint32_t>(data);
     pkt->makeAtomicResponse();
@@ -192,640 +223,427 @@ Tick
 HDLcd::write(PacketPtr pkt)
 {
     assert(pkt->getAddr() >= pioAddr &&
-           pkt->getAddr() < pioAddr + pioSize &&
-           pkt->getSize() == 4);
+           pkt->getAddr() < pioAddr + pioSize);
+
+    const Addr daddr(pkt->getAddr() - pioAddr);
+    panic_if(pkt->getSize() != 4,
+             "Unhandled read size (address: 0x.4x, size: %u)",
+             daddr, pkt->getSize());
+    const uint32_t data(pkt->get<uint32_t>());
+    DPRINTF(HDLcd, "write register 0x%04x: 0x%x\n", daddr, data);
 
-    const uint32_t data = pkt->get<uint32_t>();
-    const Addr daddr = pkt->getAddr() - pioAddr;
+    writeReg(daddr, data);
 
-    DPRINTF(HDLcd, "write register BASE+%0x04x <= 0x%08x\n", daddr,
-            pkt->get<uint32_t>());
+    pkt->makeAtomicResponse();
+    return pioDelay;
+}
 
-    switch (daddr) {
+uint32_t
+HDLcd::readReg(Addr offset)
+{
+    switch (offset) {
+      case Version: return version;
+
+      case Int_RawStat: return int_rawstat;
+      case Int_Clear:
+        panic("HDLCD INT_CLEAR register is Write-Only\n");
+      case Int_Mask: return int_mask;
+      case Int_Status: return intStatus();
+
+      case Fb_Base: return fb_base;
+      case Fb_Line_Length: return fb_line_length;
+      case Fb_Line_Count: return fb_line_count;
+      case Fb_Line_Pitch: return fb_line_pitch;
+      case Bus_Options: return bus_options;
+
+      case V_Sync: return v_sync;
+      case V_Back_Porch: return v_back_porch;
+      case V_Data: return v_data;
+      case V_Front_Porch: return v_front_porch;
+      case H_Sync: return h_sync;
+      case H_Back_Porch: return h_back_porch;
+      case H_Data: return h_data;
+      case H_Front_Porch: return h_front_porch;
+      case Polarities: return polarities;
+
+      case Command: return command;
+      case Pixel_Format: return pixel_format;
+      case Red_Select: return red_select;
+      case Green_Select: return green_select;
+      case Blue_Select: return blue_select;
+
+      default:
+        panic("Tried to read HDLCD register that doesn't  exist\n", offset);
+    }
+}
+
+void
+HDLcd::writeReg(Addr offset, uint32_t value)
+{
+    switch (offset) {
       case Version:
         panic("HDLCD VERSION register is read-Only\n");
-        break;
+
       case Int_RawStat:
-        int_rawstat = data;
-        break;
+        intRaise(value);
+        return;
       case Int_Clear:
-        int_clear = data;
-        break;
+        intClear(value);
+        return;
       case Int_Mask:
-        int_mask = data;
-        break;
+        intMask(value);
+        return;
       case Int_Status:
         panic("HDLCD INT_STATUS register is read-Only\n");
         break;
+
       case Fb_Base:
-        fb_base = data;
-        DPRINTF(HDLcd, "HDLCD Frame Buffer located at addr 0x%08x\n", fb_base);
-        break;
+        fb_base = value;
+        return;
+
       case Fb_Line_Length:
-        fb_line_length = data;
-        DPRINTF(HDLcd, "HDLCD res = %d x %d\n", width(), height());
-        break;
+        fb_line_length = value;
+        return;
+
       case Fb_Line_Count:
-        fb_line_count = data;
-        DPRINTF(HDLcd, "HDLCD res = %d x %d\n", width(), height());
-        break;
+        fb_line_count = value;
+        return;
+
       case Fb_Line_Pitch:
-        fb_line_pitch = data;
-        break;
+        fb_line_pitch = value;
+        return;
+
       case Bus_Options: {
-        BusOptsReg old_bus_options;
-        old_bus_options = bus_options;
-        bus_options = data;
-        if (bus_options.max_outstanding != old_bus_options.max_outstanding)
-            DPRINTF(HDLcd,
-                "Changing HDLcd outstanding dma transactions from %d to %d\n",
-                old_bus_options.max_outstanding, bus_options.max_outstanding);
-        if (bus_options.burst_len != old_bus_options.burst_len)
-            DPRINTF(HDLcd,
-                "Changing HDLcd dma burst length from %d bytes to %d bytes\n",
-                old_bus_options.burst_len, bus_options.burst_len); }
-        break;
+          const BusOptsReg old_bus_options(bus_options);
+          bus_options = value;
+
+          if (bus_options.max_outstanding != old_bus_options.max_outstanding) {
+              DPRINTF(HDLcd,
+                      "Changing HDLcd outstanding DMA transactions: %d -> %d\n",
+                      old_bus_options.max_outstanding,
+                      bus_options.max_outstanding);
+
+          }
+
+          if (bus_options.burst_len != old_bus_options.burst_len) {
+              DPRINTF(HDLcd,
+                      "Changing HDLcd DMA burst flags: 0x%x -> 0x%x\n",
+                      old_bus_options.burst_len, bus_options.burst_len);
+          }
+      } return;
+
       case V_Sync:
-        v_sync = data;
-        break;
+        v_sync = value;
+        return;
       case V_Back_Porch:
-        v_back_porch = data;
-        break;
+        v_back_porch = value;
+        return;
       case V_Data:
-        v_data = data;
-        break;
+        v_data = value;
+        return;
       case V_Front_Porch:
-        v_front_porch = data;
-        break;
+        v_front_porch = value;
+        return;
+
       case H_Sync:
-        h_sync = data;
-        break;
+        h_sync = value;
+        return;
       case H_Back_Porch:
-        h_back_porch = data;
-        break;
+        h_back_porch = value;
+        return;
       case H_Data:
-        h_data = data;
-        break;
+        h_data = value;
+        return;
       case H_Front_Porch:
-        h_front_porch = data;
-        break;
+        h_front_porch = value;
+        return;
+
       case Polarities:
-        polarities = data;
-        break;
+        polarities = value;
+        return;
+
       case Command: {
-        CommandReg new_command;
-        new_command = data;
-        if (new_command.enable != command.enable) {
-            DPRINTF(HDLcd, "HDLCD switched %s\n",
-                    new_command.enable==0 ? "off" : "on");
-            if (new_command.enable) {
-                doUpdateParams = true;
-                if (!frameUnderway) {
-                    schedule(startFrameEvent, clockEdge());
-                }
-            }
-        }
-        command = new_command; }
-        break;
+          const CommandReg new_command(value);
+
+          if (new_command.enable != command.enable) {
+              DPRINTF(HDLcd, "HDLCD switched %s\n",
+                      new_command.enable ? "on" : "off");
+
+              if (new_command.enable) {
+                  cmdEnable();
+              } else {
+                  cmdDisable();
+              }
+          }
+          command = new_command;
+      } return;
+
       case Pixel_Format:
-        pixel_format = data;
-        DPRINTF(HDLcd, "HDLCD res = %d x %d\n", width(), height());
-        DPRINTF(HDLcd, "HDLCD bytes per pixel = %d\n", bytesPerPixel());
-        DPRINTF(HDLcd, "HDLCD endianness = %s\n",
-                pixel_format.big_endian ? "big" : "little");
-        break;
+        pixel_format = value;
+        return;
+
       case Red_Select:
-        red_select = data;
-        break;
+        red_select = value;
+        return;
       case Green_Select:
-        green_select = data;
-        break;
+        green_select = value;
+        return;
       case Blue_Select:
-        blue_select = data;
-        break;
+        blue_select = value;
+        return;
+
       default:
-        panic("Tried to write HDLCD register that doesn't exist\n", daddr);
-        break;
+        panic("Tried to write HDLCD register that doesn't exist\n", offset);
+        return;
     }
-
-    pkt->makeAtomicResponse();
-    return pioDelay;
 }
 
-void
-HDLcd::updateVideoParams(bool unserializing = false)
+PixelConverter
+HDLcd::pixelConverter() const
 {
-    const uint16_t bpp M5_VAR_USED = bytesPerPixel() << 3;
-
-    // Workaround configuration bugs where multiple display
-    // controllers are attached to the same VNC server by reattaching
-    // enabled devices. This isn't ideal, but works as long as only
-    // one display controller is active at a time.
-    if (command.enable && vnc)
-        vnc->setFrameBuffer(&fb);
-
-    // updating these parameters while LCD is enabled is not supported
-    if (frameUnderway && !unserializing)
-        panic("Attempting to change some HDLCD parameters while the controller"
-                " is active is not allowed");
-
-    // resize the virtualDisplayBuffer unless we are unserializing - it may
-    //   have changed size
-    // there must be no outstanding DMA transactions for this to work
-    if (!unserializing) {
-        assert(dmaPendingNum == 0);
-
-        virtualDisplayBuffer.resize(bytesPerPixel() * area());
-        fb.resize(width(), height());
-        fb.clear();
-
-        std::fill(virtualDisplayBuffer.begin(), virtualDisplayBuffer.end(),
-                  0);
+    ByteOrder byte_order(
+        pixel_format.big_endian ? BigEndianByteOrder : LittleEndianByteOrder);
+
+    /* Some Linux kernels have a broken driver that swaps the red and
+     * blue color select registers. */
+    if (!workaroundSwapRB) {
+        return PixelConverter(
+            pixel_format.bytes_per_pixel + 1,
+            red_select.offset, green_select.offset, blue_select.offset,
+            red_select.size, green_select.size, blue_select.size,
+            byte_order);
+    } else {
+        return PixelConverter(
+            pixel_format.bytes_per_pixel + 1,
+            blue_select.offset, green_select.offset, red_select.offset,
+            blue_select.size, green_select.size, red_select.size,
+            byte_order);
     }
+}
 
-    DPRINTF(HDLcd, "bpp = %d\n", bpp);
-    DPRINTF(HDLcd, "display size = %d x %d\n", width(), height());
-#if TRACING_ON
-    const size_t totalLinesPerFrame = v_back_porch.val + 1 +
-                                      v_data.val + 1 +
-                                      v_front_porch.val + 1 +
-                                      v_sync.val + 1;
-    const double fps = (double)SimClock::Frequency /
-            (double)(PClksPerLine() * totalLinesPerFrame * pixelClock);
-#endif
-    DPRINTF(HDLcd, "simulated refresh rate ~ %.1ffps generating ~ %.1fMB/s "
-            "traffic ([%.1fMHz, T=%d sim clocks] pclk, %d bpp => %.1fMB/s peak requirement)\n",
-            fps,
-            fps * virtualDisplayBuffer.size() / 1024 / 1024,
-            (double)SimClock::Frequency / pixelClock / 1000000.0,
-            pixelClock,
-            bpp,
-            (double)(SimClock::Frequency / pixelClock * (bpp / 8)) / 1024 / 1024);
+DisplayTimings
+HDLcd::displayTimings() const
+{
+    return DisplayTimings(
+        h_data.val + 1, v_data.val + 1,
+        h_back_porch.val + 1, h_sync.val + 1, h_front_porch.val + 1,
+        v_back_porch.val + 1, v_sync.val + 1, v_front_porch.val + 1);
 }
 
 void
-HDLcd::startFrame()
+HDLcd::createDmaEngine()
 {
-    // 0. Check that we are in the appropriate state
-    assert(!frameUnderway);
-    if (!command.enable)
+    if (bus_options.max_outstanding == 0) {
+        warn("Maximum number of outstanding DMA transfers set to 0.");
         return;
-    DPRINTF(HDLcd, "Frame read started\n");
-    if (doUpdateParams) {
-        updateVideoParams();
-        doUpdateParams = false;
     }
-    frameUnderway = true;
-    assert(!virtualDisplayBuffer.empty());
-    assert(pixelBufferSize == 0);
-    assert(dmaBytesInFlight == 0);
-    assert(dmaPendingNum == 0);
-    assert(dmaDoneEventFree.size() == dmaDoneEventAll.size());
-    assert(!renderPixelEvent.scheduled());
-    // currently only support positive line pitches equal to the line length
-    assert(width() * bytesPerPixel() == fb_line_pitch);
-
-    // 1. Start DMA'ing the frame; subsequent transactions created as we go
-    dmaCurAddr = dmaStartAddr = fb_base;
-    dmaMaxAddr = static_cast<Addr>(width() * height() * bytesPerPixel()) +
-                    dmaCurAddr;
-    frameReadStartTime = curTick();
-    pixelIndex = 0;
-    frameUnderrun = false;
-    fillPixelBuffer();
-
-    // 2. Schedule first pixelclock read; subsequent reads generated as we go
-    Tick firstPixelReadTick = curTick() + pixelClock * (
-                                  PClksPerLine() * (v_sync.val + 1 +
-                                                    v_back_porch.val + 1) +
-                                  h_sync.val + 1 +
-                                  h_back_porch.val + 1);
-    schedule(renderPixelEvent, firstPixelReadTick);
+
+    const uint32_t dma_burst_flags(bus_options.burst_len);
+    const uint32_t dma_burst_len(
+        dma_burst_flags ?
+        (1UL << (findMsbSet(dma_burst_flags) - 1)) :
+        MAX_BURST_LEN);
+    // Some drivers seem to set the DMA line count incorrectly. This
+    // could either be a driver bug or a specification bug. Unlike for
+    // timings, the specification does not require 1 to be added to
+    // the DMA engine's line count.
+    const uint32_t dma_lines(
+        fb_line_count + (workaroundDmaLineCount ? 1 : 0));
+
+    dmaEngine.reset(new DmaEngine(
+                        *this, pixelBufferSize,
+                        AXI_PORT_WIDTH * dma_burst_len,
+                        bus_options.max_outstanding,
+                        fb_line_length, fb_line_pitch, dma_lines));
 }
 
 void
-HDLcd::fillPixelBuffer()
+HDLcd::cmdEnable()
 {
-    // - am I under the LCD dma transaction total?
-    // - do I have more data to transfer?
-    // - have I not yet underrun for this frame?
-    // - is there room to put the data in the pixel buffer including any
-    //   outstanding dma transfers in flight?
-    while ((dmaPendingNum < maxOutstandingDma()) &&
-           (dmaMaxAddr > dmaCurAddr) &&
-           !frameUnderrun &&
-           bytesFreeInPixelBuffer() > dmaBurstLength() * AXI_PORT_WIDTH) {
-        // try largest transaction size allowed first but switch to smaller
-        // sizes for trailing bytes
-        size_t transaction_size = dmaBurstLength() * AXI_PORT_WIDTH;
-        while (transaction_size > (dmaMaxAddr - dmaCurAddr))
-            transaction_size >>= 1;
-        assert(transaction_size > 0);
-
-        // concurrent dma reads need different dma done events
-        // due to assertion in scheduling state
-        ++dmaPendingNum;
-
-        assert(!dmaDoneEventFree.empty());
-        DmaDoneEvent *event(dmaDoneEventFree.back());
-        dmaDoneEventFree.pop_back();
-        assert(event);
-        assert(!event->scheduled());
-
-        // We use a uncachable request here because the requests from the CPU
-        // will be uncacheable as well. If we have uncacheable and cacheable
-        // requests in the memory system for the same address it won't be
-        // pleased
-        uint8_t *const dma_dst(
-            virtualDisplayBuffer.data() + dmaCurAddr - dmaStartAddr);
-        event->setTransactionSize(transaction_size);
-        dmaPort.dmaAction(MemCmd::ReadReq, dmaCurAddr, transaction_size, event,
-                          dma_dst, 0, Request::UNCACHEABLE);
-        dmaCurAddr += transaction_size;
-        dmaBytesInFlight += transaction_size;
-    }
+    createDmaEngine();
+    conv = pixelConverter();
+    pixelPump.start(displayTimings());
 }
 
 void
-HDLcd::renderPixel()
+HDLcd::cmdDisable()
 {
-    // try to handle multiple pixels at a time; doing so reduces the accuracy
-    //   of the underrun detection but lowers simulation overhead
-    const size_t count = 32;
-    assert(width() % count == 0); // not set up to handle trailing pixels
-
-    // have we underrun on this frame anytime before?
-    if (frameUnderrun) {
-        // the LCD controller gives up on a frame if an underrun occurs and
-        //   resumes regular operation on the next frame
-        pixelBufferSize = 0;
-    } else {
-        // did we underrun on this set of pixels?
-        if (pixelBufferSize < bytesPerPixel() * count) {
-            warn("HDLcd controller buffer underrun\n");
-            frameUnderrun = true;
-            int_rawstat.underrun = 1;
-            if (!intEvent.scheduled())
-                schedule(intEvent, clockEdge());
-        } else {
-            // emulate the pixel read from the internal buffer
-            pixelBufferSize -= bytesPerPixel() * count;
-        }
-    }
+    pixelPump.stop();
+    dmaEngine->abortFrame();
+}
 
-    // the DMA may have previously stalled due to the buffer being full;
-    //   give it a kick; it knows not to fill if at end of frame, underrun, etc
-    if (!fillPixelBufferEvent.scheduled())
-        schedule(fillPixelBufferEvent, clockEdge());
-
-    // schedule the next pixel read according to where it is in the frame
-    pixelIndex += count;
-    assert(pixelIndex <= width() * height());
-    size_t x = pixelIndex % width();
-    Tick nextEventTick = curTick();
-    if (x == 0) {
-        // start of new line
-        nextEventTick += pixelClock * ((h_front_porch.val + 1) +
-                                       (h_back_porch.val + 1) +
-                                       (h_sync.val + 1));
-        if (pixelIndex == width() * height()) {
-            // end of frame
-            nextEventTick += PClksPerLine() * (v_front_porch.val + 1) *
-                             pixelClock;
-            schedule(endFrameEvent, nextEventTick);
-            return;
-        }
+bool
+HDLcd::pxlNext(Pixel &p)
+{
+    uint8_t pixel_data[MAX_PIXEL_SIZE];
+    assert(conv.length <= sizeof(pixel_data));
+    if (dmaEngine->tryGet(pixel_data, conv.length)) {
+        p = conv.toPixel(pixel_data);
+        return true;
     } else {
-        nextEventTick += pixelClock * count;
+        return false;
     }
-
-    schedule(renderPixelEvent, nextEventTick);
 }
 
-PixelConverter
-HDLcd::pixelConverter() const
+void
+HDLcd::pxlVSyncBegin()
 {
-    ByteOrder byte_order(
-        pixel_format.big_endian ? BigEndianByteOrder : LittleEndianByteOrder);
+    DPRINTF(HDLcd, "Raising VSYNC interrupt.\n");
+    intRaise(INT_VSYNC);
+}
 
-    /* Some Linux kernels have a broken driver that swaps the red and
-     * blue color select registers. */
-    if (!workaround_swap_rb) {
-        return PixelConverter(
-            bytesPerPixel(),
-            red_select.offset, green_select.offset, blue_select.offset,
-            red_select.size, green_select.size, blue_select.size,
-            byte_order);
-    } else {
-        return PixelConverter(
-            bytesPerPixel(),
-            blue_select.offset, green_select.offset, red_select.offset,
-            blue_select.size, green_select.size, red_select.size,
-            byte_order);
-    }
+void
+HDLcd::pxlVSyncEnd()
+{
+    DPRINTF(HDLcd, "End of VSYNC, starting DMA engine\n");
+    dmaEngine->startFrame(fb_base);
 }
 
 void
-HDLcd::endFrame() {
-    assert(pixelBufferSize == 0);
-    assert(dmaPendingNum == 0);
-    assert(dmaBytesInFlight == 0);
-    assert(dmaDoneEventFree.size() == dmaDoneEventAll.size());
+HDLcd::pxlUnderrun()
+{
+    DPRINTF(HDLcd, "Buffer underrun, stopping DMA fill.\n");
+    intRaise(INT_UNDERRUN);
+    dmaEngine->abortFrame();
+}
 
-    fb.copyIn(virtualDisplayBuffer, pixelConverter());
+void
+HDLcd::pxlFrameDone()
+{
+    DPRINTF(HDLcd, "Reached end of last visible line.\n");
+
+    if (dmaEngine->size()) {
+        warn("HDLCD %u bytes still in FIFO after frame: Ensure that DMA "
+             "and PixelPump configuration is consistent\n",
+             dmaEngine->size());
+        dmaEngine->dumpSettings();
+        pixelPump.dumpSettings();
+    }
 
     if (vnc)
         vnc->setDirty();
 
     if (enableCapture) {
-        if (!pic)
-            pic = simout.create(csprintf("%s.framebuffer.bmp", sys->name()), true);
+        if (!pic) {
+            pic = simout.create(
+                csprintf("%s.framebuffer.bmp", sys->name()),
+                true);
+        }
 
         assert(pic);
         pic->seekp(0);
         bmp.write(*pic);
     }
-
-    // start the next frame
-    frameUnderway = false;
-    startFrame();
 }
 
 void
-HDLcd::dmaDone(DmaDoneEvent *event)
+HDLcd::setInterrupts(uint32_t ints, uint32_t mask)
 {
-    const size_t transactionLength = event->getTransactionSize();
-    assert(pixelBufferSize + transactionLength < PIXEL_BUFFER_CAPACITY);
-    assert(dmaCurAddr <= dmaMaxAddr);
-
-    dmaDoneEventFree.push_back(event);
-    --dmaPendingNum;
-    assert(MAX_OUTSTANDING_DMA_REQ_CAPACITY - dmaDoneEventFree.size() ==
-            dmaPendingNum);
-
-    // add the data to the pixel buffer
-    dmaBytesInFlight -= transactionLength;
-    pixelBufferSize += transactionLength;
-
-    // schedule another dma transaction if:
-    // - we're not done reading the frame
-    // - there is sufficient room in the pixel buffer for another transaction
-    // - another fillPixelBufferEvent is not already scheduled
-    const size_t targetTransSize = dmaBurstLength() * AXI_PORT_WIDTH;
-    if ((dmaCurAddr < dmaMaxAddr) &&
-        (bytesFreeInPixelBuffer() + targetTransSize < PIXEL_BUFFER_CAPACITY) &&
-        !fillPixelBufferEvent.scheduled()) {
-        schedule(fillPixelBufferEvent, clockEdge());
+    const bool old_ints(intStatus());
+
+    int_mask = mask;
+    int_rawstat = ints;
+
+    if (!old_ints && intStatus()) {
+        gic->sendInt(intNum);
+    } else if (old_ints && !intStatus()) {
+        gic->clearInt(intNum);
     }
 }
 
-void
-HDLcd::serialize(CheckpointOut &cp) const
+HDLcd::DmaEngine::DmaEngine(HDLcd &_parent, size_t size,
+          unsigned request_size, unsigned max_pending,
+          size_t line_size, ssize_t line_pitch, unsigned num_lines)
+    : DmaReadFifo(
+        _parent.dmaPort, size, request_size, max_pending,
+        Request::UNCACHEABLE),
+      parent(_parent),
+      lineSize(line_size), linePitch(line_pitch), numLines(num_lines),
+      nextLineAddr(0)
 {
-    DPRINTF(HDLcd, "Serializing ARM HDLCD\n");
-
-    const uint32_t version_serial = version;
-    SERIALIZE_SCALAR(version_serial);
-    const uint32_t int_rawstat_serial = int_rawstat;
-    SERIALIZE_SCALAR(int_rawstat_serial);
-    const uint32_t int_clear_serial = int_clear;
-    SERIALIZE_SCALAR(int_clear_serial);
-    const uint32_t int_mask_serial = int_mask;
-    SERIALIZE_SCALAR(int_mask_serial);
-    const uint32_t int_status_serial = int_status;
-    SERIALIZE_SCALAR(int_status_serial);
+}
 
-    SERIALIZE_SCALAR(fb_base);
-    SERIALIZE_SCALAR(fb_line_length);
+void
+HDLcd::DmaEngine::serialize(CheckpointOut &cp) const
+{
+    DmaReadFifo::serialize(cp);
 
-    const uint32_t fb_line_count_serial = fb_line_count;
-    SERIALIZE_SCALAR(fb_line_count_serial);
+    SERIALIZE_SCALAR(nextLineAddr);
+    SERIALIZE_SCALAR(frameEnd);
+}
 
-    SERIALIZE_SCALAR(fb_line_pitch);
+void
+HDLcd::DmaEngine::unserialize(CheckpointIn &cp)
+{
+    DmaReadFifo::unserialize(cp);
 
-    const uint32_t bus_options_serial = bus_options;
-    SERIALIZE_SCALAR(bus_options_serial);
-    const uint32_t v_sync_serial = v_sync;
-    SERIALIZE_SCALAR(v_sync_serial);
-    const uint32_t v_back_porch_serial = v_back_porch;
-    SERIALIZE_SCALAR(v_back_porch_serial);
-    const uint32_t v_data_serial = v_data;
-    SERIALIZE_SCALAR(v_data_serial);
-    const uint32_t v_front_porch_serial = v_front_porch;
-    SERIALIZE_SCALAR(v_front_porch_serial);
-    const uint32_t h_sync_serial = h_sync;
-    SERIALIZE_SCALAR(h_sync_serial);
-    const uint32_t h_back_porch_serial = h_back_porch;
-    SERIALIZE_SCALAR(h_back_porch_serial);
-    const uint32_t h_data_serial = h_data;
-    SERIALIZE_SCALAR(h_data_serial);
-    const uint32_t h_front_porch_serial = h_front_porch;
-    SERIALIZE_SCALAR(h_front_porch_serial);
-    const uint32_t polarities_serial = polarities;
-    SERIALIZE_SCALAR(polarities_serial);
-    const uint32_t command_serial = command;
-    SERIALIZE_SCALAR(command_serial);
-    const uint32_t pixel_format_serial = pixel_format;
-    SERIALIZE_SCALAR(pixel_format_serial);
-    const uint32_t red_select_serial = red_select;
-    SERIALIZE_SCALAR(red_select_serial);
-    const uint32_t green_select_serial = green_select;
-    SERIALIZE_SCALAR(green_select_serial);
-    const uint32_t blue_select_serial = blue_select;
-    SERIALIZE_SCALAR(blue_select_serial);
-
-    SERIALIZE_SCALAR(frameReadStartTime);
-    SERIALIZE_SCALAR(dmaStartAddr);
-    SERIALIZE_SCALAR(dmaCurAddr);
-    SERIALIZE_SCALAR(dmaMaxAddr);
-    SERIALIZE_SCALAR(dmaPendingNum);
-    SERIALIZE_SCALAR(frameUnderrun);
-
-    arrayParamOut(cp, "virtualDisplayBuffer", virtualDisplayBuffer);
-
-    SERIALIZE_SCALAR(pixelBufferSize);
-    SERIALIZE_SCALAR(pixelIndex);
-    SERIALIZE_SCALAR(doUpdateParams);
-    SERIALIZE_SCALAR(frameUnderway);
-    SERIALIZE_SCALAR(dmaBytesInFlight);
-
-    Tick start_event_time = 0;
-    Tick end_event_time = 0;
-    Tick render_pixel_event_time = 0;
-    Tick fill_pixel_buffer_event_time = 0;
-    Tick int_event_time = 0;
-    if (startFrameEvent.scheduled())
-        start_event_time = startFrameEvent.when();
-    if (endFrameEvent.scheduled())
-        end_event_time = endFrameEvent.when();
-    if (renderPixelEvent.scheduled())
-        render_pixel_event_time = renderPixelEvent.when();
-    if (fillPixelBufferEvent.scheduled())
-        fill_pixel_buffer_event_time = fillPixelBufferEvent.when();
-    if (intEvent.scheduled())
-        int_event_time = intEvent.when();
-    SERIALIZE_SCALAR(start_event_time);
-    SERIALIZE_SCALAR(end_event_time);
-    SERIALIZE_SCALAR(render_pixel_event_time);
-    SERIALIZE_SCALAR(fill_pixel_buffer_event_time);
-    SERIALIZE_SCALAR(int_event_time);
-
-    vector<Tick> dma_done_event_tick(MAX_OUTSTANDING_DMA_REQ_CAPACITY);
-    vector<size_t> dma_done_event_burst_len(MAX_OUTSTANDING_DMA_REQ_CAPACITY);
-    for (int x = 0; x < MAX_OUTSTANDING_DMA_REQ_CAPACITY; ++x) {
-        dma_done_event_tick[x] = dmaDoneEventAll[x].scheduled() ?
-            dmaDoneEventAll[x].when() : 0;
-        dma_done_event_burst_len[x] = dmaDoneEventAll[x].scheduled() ?
-            dmaDoneEventAll[x].getTransactionSize() : 0;
-    }
-    arrayParamOut(cp, "dma_done_event_tick", dma_done_event_tick);
-    arrayParamOut(cp, "dma_done_event_burst_length", dma_done_event_burst_len);
+    UNSERIALIZE_SCALAR(nextLineAddr);
+    UNSERIALIZE_SCALAR(frameEnd);
 }
 
 void
-HDLcd::unserialize(CheckpointIn &cp)
+HDLcd::DmaEngine::startFrame(Addr fb_base)
 {
-    uint32_t version_serial, int_rawstat_serial, int_clear_serial,
-            int_mask_serial, int_status_serial, fb_line_count_serial,
-            bus_options_serial, v_sync_serial, v_back_porch_serial,
-            v_data_serial, v_front_porch_serial, h_sync_serial,
-            h_back_porch_serial, h_data_serial, h_front_porch_serial,
-            polarities_serial, command_serial, pixel_format_serial,
-            red_select_serial, green_select_serial, blue_select_serial;
-
-    DPRINTF(HDLcd, "Unserializing ARM HDLCD\n");
-
-    UNSERIALIZE_SCALAR(version_serial);
-    version = version_serial;
-    UNSERIALIZE_SCALAR(int_rawstat_serial);
-    int_rawstat = int_rawstat_serial;
-    UNSERIALIZE_SCALAR(int_clear_serial);
-    int_clear = int_clear_serial;
-    UNSERIALIZE_SCALAR(int_mask_serial);
-    int_mask = int_mask_serial;
-    UNSERIALIZE_SCALAR(int_status_serial);
-    int_status = int_status_serial;
+    nextLineAddr = fb_base;
+    frameEnd = fb_base + numLines * linePitch;
 
-    UNSERIALIZE_SCALAR(fb_base);
-    UNSERIALIZE_SCALAR(fb_line_length);
+    startFill(nextLineAddr, lineSize);
+}
 
-    UNSERIALIZE_SCALAR(fb_line_count_serial);
-    fb_line_count = fb_line_count_serial;
+void
+HDLcd::DmaEngine::abortFrame()
+{
+    nextLineAddr = frameEnd;
+    stopFill();
+    flush();
+}
 
-    UNSERIALIZE_SCALAR(fb_line_pitch);
 
-    UNSERIALIZE_SCALAR(bus_options_serial);
-    bus_options = bus_options_serial;
-    UNSERIALIZE_SCALAR(v_sync_serial);
-    v_sync = v_sync_serial;
-    UNSERIALIZE_SCALAR(v_back_porch_serial);
-    v_back_porch = v_back_porch_serial;
-    UNSERIALIZE_SCALAR(v_data_serial);
-    v_data = v_data_serial;
-    UNSERIALIZE_SCALAR(v_front_porch_serial);
-    v_front_porch = v_front_porch_serial;
-    UNSERIALIZE_SCALAR(h_sync_serial);
-    h_sync = h_sync_serial;
-    UNSERIALIZE_SCALAR(h_back_porch_serial);
-    h_back_porch = h_back_porch_serial;
-    UNSERIALIZE_SCALAR(h_data_serial);
-    h_data = h_data_serial;
-    UNSERIALIZE_SCALAR(h_front_porch_serial);
-    h_front_porch = h_front_porch_serial;
-    UNSERIALIZE_SCALAR(polarities_serial);
-    polarities = polarities_serial;
-    UNSERIALIZE_SCALAR(command_serial);
-    command = command_serial;
-    UNSERIALIZE_SCALAR(pixel_format_serial);
-    pixel_format = pixel_format_serial;
-    UNSERIALIZE_SCALAR(red_select_serial);
-    red_select = red_select_serial;
-    UNSERIALIZE_SCALAR(green_select_serial);
-    green_select = green_select_serial;
-    UNSERIALIZE_SCALAR(blue_select_serial);
-    blue_select = blue_select_serial;
-
-    UNSERIALIZE_SCALAR(frameReadStartTime);
-    UNSERIALIZE_SCALAR(dmaStartAddr);
-    UNSERIALIZE_SCALAR(dmaCurAddr);
-    UNSERIALIZE_SCALAR(dmaMaxAddr);
-    UNSERIALIZE_SCALAR(dmaPendingNum);
-    UNSERIALIZE_SCALAR(frameUnderrun);
-    UNSERIALIZE_SCALAR(dmaBytesInFlight);
-
-    arrayParamIn(cp, "virtualDisplayBuffer", virtualDisplayBuffer);
-
-    UNSERIALIZE_SCALAR(pixelBufferSize);
-    UNSERIALIZE_SCALAR(pixelIndex);
-    UNSERIALIZE_SCALAR(doUpdateParams);
-    UNSERIALIZE_SCALAR(frameUnderway);
-
-    Tick start_event_time = 0;
-    Tick end_event_time = 0;
-    Tick render_pixel_event_time = 0;
-    Tick fill_pixel_buffer_event_time = 0;
-    Tick int_event_time = 0;
-    UNSERIALIZE_SCALAR(start_event_time);
-    UNSERIALIZE_SCALAR(end_event_time);
-    UNSERIALIZE_SCALAR(render_pixel_event_time);
-    UNSERIALIZE_SCALAR(fill_pixel_buffer_event_time);
-    UNSERIALIZE_SCALAR(int_event_time);
-    if (start_event_time)
-        schedule(startFrameEvent, start_event_time);
-    if (end_event_time)
-        schedule(endFrameEvent, end_event_time);
-    if (render_pixel_event_time)
-        schedule(renderPixelEvent, render_pixel_event_time);
-    if (fill_pixel_buffer_event_time)
-        schedule(fillPixelBufferEvent, fill_pixel_buffer_event_time);
-    if (int_event_time)
-        schedule(intEvent, int_event_time);
-
-    vector<Tick> dma_done_event_tick(MAX_OUTSTANDING_DMA_REQ_CAPACITY);
-    vector<Tick> dma_done_event_burst_len(MAX_OUTSTANDING_DMA_REQ_CAPACITY);
-    arrayParamIn(cp, "dma_done_event_tick", dma_done_event_tick);
-    arrayParamIn(cp, "dma_done_event_burst_length", dma_done_event_burst_len);
-    dmaDoneEventFree.clear();
-    for (int x = 0; x < MAX_OUTSTANDING_DMA_REQ_CAPACITY; ++x) {
-        if (dma_done_event_tick[x]) {
-            dmaDoneEventAll[x].setTransactionSize(dma_done_event_burst_len[x]);
-            schedule(dmaDoneEventAll[x], dma_done_event_tick[x]);
-        } else
-            dmaDoneEventFree.push_back(&dmaDoneEventAll[x]);
-    }
-    assert(MAX_OUTSTANDING_DMA_REQ_CAPACITY - dmaDoneEventFree.size() == dmaPendingNum);
-
-    if (frameUnderway) {
-        updateVideoParams(true);
-        fb.resize(width(), height());
-        fb.copyIn(virtualDisplayBuffer, pixelConverter());
-        if (vnc)
-            vnc->setDirty();
-    }
+void
+HDLcd::DmaEngine::dumpSettings()
+{
+    inform("DMA line size: %u bytes", lineSize);
+    inform("DMA line pitch: %i bytes", linePitch);
+    inform("DMA num lines: %u", numLines);
 }
 
 void
-HDLcd::generateInterrupt()
+HDLcd::DmaEngine::onEndOfBlock()
 {
-    int_status = int_rawstat & int_mask;
-    DPRINTF(HDLcd, "Generate Interrupt: int_rawstat=0x%08x int_mask=0x%08x "
-            "int_status=0x%08x\n",
-            (uint32_t)int_rawstat, (uint32_t)int_mask, (uint32_t)int_status);
+    if (nextLineAddr == frameEnd)
+        // We're done with this frame. Ignore calls to this method
+        // until the next frame has been started.
+        return;
 
-    if (int_status != 0) {
-        gic->sendInt(intNum);
-        DPRINTF(HDLcd, " -- Generated\n");
-    }
+    nextLineAddr += linePitch;
+    if (nextLineAddr != frameEnd)
+        startFill(nextLineAddr, lineSize);
 }
 
-AddrRangeList
-HDLcd::getAddrRanges() const
+void
+HDLcd::DmaEngine::onIdle()
 {
-    AddrRangeList ranges;
-    ranges.push_back(RangeSize(pioAddr, pioSize));
-    return ranges;
+    parent.intRaise(INT_DMA_END);
 }
 
+void
+HDLcd::PixelPump::dumpSettings()
+{
+    const DisplayTimings &t(timings());
+
+    inform("PixelPump width: %u", t.width);
+    inform("PixelPump height: %u", t.height);
+
+    inform("PixelPump horizontal back porch: %u", t.hBackPorch);
+    inform("PixelPump horizontal fron porch: %u", t.hFrontPorch);
+    inform("PixelPump horizontal fron porch: %u", t.hSync);
+
+    inform("PixelPump vertical back porch: %u", t.vBackPorch);
+    inform("PixelPump vertical fron porch: %u", t.vFrontPorch);
+    inform("PixelPump vertical fron porch: %u", t.vSync);
+}
+
+
 HDLcd *
 HDLcdParams::create()
 {
diff --git a/src/dev/arm/hdlcd.hh b/src/dev/arm/hdlcd.hh
index 1396c9a8b..3a83c6ad4 100644
--- a/src/dev/arm/hdlcd.hh
+++ b/src/dev/arm/hdlcd.hh
@@ -35,6 +35,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  * Authors: Chris Emmons
+ *          Andreas Sandberg
  */
 
 
@@ -61,22 +62,15 @@
  * content of an underrun frame.
  *
  * KNOWN ISSUES
- * 1.  The default kernel driver used in testing sets the line count to one
- * less than the expected 768.  However, it also sets the v_count to 767.
- * The controller specifies that 1 should be added to v_count but does not
- * specify adding 1 to the line count.  The driver is probably wrong.
- * However, to sync these two numbers up, this model uses fb_line_count and
- * fb_line_length rather than using v_data or h_data values to determine the
- * width and height of the frame; those values are ignored.
- * 2.  The HDLcd is implemented here as an AmbaDmaDevice, but it doesn't have
- * an AMBA ID as far as I know.  That is the only bit of the AmbaDmaDevice
- * interface that is irrelevant to it, so a fake AMBA ID is used for now.
- * I didn't think inserting an extra layer of hierachy between AmbaDmaDevice
- * and DmaDevice would be helpful to anyone else, but that may be the right
- * answer.
- * 3.  The internal buffer size is either 1 or 2 KB depending on which
- * specification is referenced for the different Versatile Express tiles.
- * This implementation uses the larger 2 KB buffer by default.
+ * <ul>
+ *   <li>The HDLcd is implemented here as an AmbaDmaDevice, but it
+ *       doesn't have an AMBA ID as far as I know.  That is the only
+ *       bit of the AmbaDmaDevice interface that is irrelevant to it,
+ *       so a fake AMBA ID is used for now.  I didn't think inserting
+ *       an extra layer of hierachy between AmbaDmaDevice and
+ *       DmaDevice would be helpful to anyone else, but that may be
+ *       the right answer.
+ * </ul>
  */
 
 #ifndef __DEV_ARM_HDLCD_HH__
@@ -88,17 +82,39 @@
 #include "base/bitmap.hh"
 #include "base/framebuffer.hh"
 #include "dev/arm/amba_device.hh"
-#include "params/HDLcd.hh"
+#include "dev/pixelpump.hh"
 #include "sim/serialize.hh"
 
 class VncInput;
+struct HDLcdParams;
+class HDLcdPixelPump;
 
 class HDLcd: public AmbaDmaDevice
 {
-  protected:
-    /** fake AMBA ID -- unused */
-    static const uint64_t AMBA_ID       = ULL(0xb105f00d00141000);
+  public:
+    HDLcd(const HDLcdParams *p);
+    ~HDLcd();
+
+    void serialize(CheckpointOut &cp) const M5_ATTR_OVERRIDE;
+    void unserialize(CheckpointIn &cp) M5_ATTR_OVERRIDE;
+
+    void drainResume() M5_ATTR_OVERRIDE;
 
+  public: // IO device interface
+    Tick read(PacketPtr pkt) M5_ATTR_OVERRIDE;
+    Tick write(PacketPtr pkt) M5_ATTR_OVERRIDE;
+
+    AddrRangeList getAddrRanges() const M5_ATTR_OVERRIDE { return addrRanges; }
+
+  protected: // Parameters
+    VncInput *vnc;
+    const bool workaroundSwapRB;
+    const bool workaroundDmaLineCount;
+    const AddrRangeList addrRanges;
+    const bool enableCapture;
+    const Addr pixelBufferSize;
+
+  protected: // Register handling
     /** ARM HDLcd register offsets */
     enum RegisterOffset {
         Version          = 0x0000,
@@ -124,27 +140,23 @@ class HDLcd: public AmbaDmaDevice
         Pixel_Format     = 0x0240,
         Red_Select       = 0x0244,
         Green_Select     = 0x0248,
-        Blue_Select      = 0x024C };
+        Blue_Select      = 0x024C,
+    };
 
     /** Reset value for Bus_Options register */
-    static const size_t BUS_OPTIONS_RESETV = 0x408;
+    static constexpr size_t BUS_OPTIONS_RESETV = 0x408;
 
     /** Reset value for Version register */
-    static const size_t VERSION_RESETV = 0x1CDC0000;
+    static constexpr size_t VERSION_RESETV = 0x1CDC0000;
 
-    /** max number of outstanding DMA requests possible */
-    static const size_t MAX_OUTSTANDING_DMA_REQ_CAPACITY = 16;
+    /** AXI port width in bytes */
+    static constexpr size_t AXI_PORT_WIDTH = 8;
 
     /** max number of beats delivered in one dma burst */
-    static const size_t MAX_BURST_LEN = 16;
+    static constexpr size_t MAX_BURST_LEN = 16;
 
-    /** size of internal buffer in bytes */
-    static const size_t PIXEL_BUFFER_CAPACITY = 2048;
-
-    /** AXI port width in bytes */
-    static const size_t AXI_PORT_WIDTH = 8;
-
-    static const size_t MAX_BURST_SIZE = MAX_BURST_LEN * AXI_PORT_WIDTH;
+    /** Maximum number of bytes per pixel */
+    static constexpr size_t MAX_PIXEL_SIZE = 4;
 
     /**
      * @name RegisterFieldLayouts
@@ -157,12 +169,10 @@ class HDLcd: public AmbaDmaDevice
         Bitfield<31,16> product_id;
     EndBitUnion(VersionReg)
 
-    BitUnion32(InterruptReg)
-        Bitfield<0> dma_end;
-        Bitfield<1> bus_error;
-        Bitfield<2> vsync;
-        Bitfield<3> underrun;
-    EndBitUnion(InterruptReg)
+    static constexpr uint32_t INT_DMA_END = (1UL << 0);
+    static constexpr uint32_t INT_BUS_ERROR = (1UL << 1);
+    static constexpr uint32_t INT_VSYNC = (1UL << 2);
+    static constexpr uint32_t INT_UNDERRUN = (1UL << 3);
 
     BitUnion32(FbLineCountReg)
         Bitfield<11,0>  fb_line_count;
@@ -217,15 +227,13 @@ class HDLcd: public AmbaDmaDevice
      * HDLCD register contents.
      */
     /**@{*/
-    VersionReg version;             /**< Version register */
-    InterruptReg int_rawstat;       /**< Interrupt raw status register */
-    InterruptReg int_clear;         /**< Interrupt clear register */
-    InterruptReg int_mask;          /**< Interrupt mask register */
-    InterruptReg int_status;        /**< Interrupt status register */
+    const VersionReg version;       /**< Version register */
+    uint32_t int_rawstat;           /**< Interrupt raw status register */
+    uint32_t int_mask;              /**< Interrupt mask register */
     uint32_t fb_base;               /**< Frame buffer base address register */
     uint32_t fb_line_length;        /**< Frame buffer Line length register */
     FbLineCountReg fb_line_count;   /**< Frame buffer Line count register */
-    uint32_t fb_line_pitch;         /**< Frame buffer Line pitch register */
+    int32_t fb_line_pitch;          /**< Frame buffer Line pitch register */
     BusOptsReg bus_options;         /**< Bus options register */
     TimingReg v_sync;               /**< Vertical sync width register */
     TimingReg v_back_porch;         /**< Vertical back porch width register */
@@ -243,274 +251,136 @@ class HDLcd: public AmbaDmaDevice
     ColorSelectReg blue_select;     /**< Blue color select register */
     /** @} */
 
-    /** Pixel clock period */
-    const Tick pixelClock;
-
-    FrameBuffer fb;
-
-    /** VNC server */
-    VncInput *vnc;
-
-    /** Helper to write out bitmaps */
-    Bitmap bmp;
-
-    /** Picture of what the current frame buffer looks like */
-    std::ostream *pic;
-
-    /**
-     * Event wrapper for dmaDone()
-     *
-     * This event call pushes its this pointer onto the freeDoneEvent vector
-     * and calls dmaDone() when triggered.  While most of the time the burst
-     * length of a transaction will be the max burst length set by the driver,
-     * any trailing bytes must be handled with smaller lengths thus requiring
-     * the configurable burst length option.
-     */
-    class DmaDoneEvent : public Event
-    {
-      private:
-        /** Reference to HDLCD that issued the corresponding DMA transaction */
-        HDLcd &obj;
-
-        /** Transaction size */
-        size_t transSize;
-
-      public:
-        /**
-         * Constructor.
-         *
-         * @param _obj HDLCD that issued the corresponding DMA transaction
-         */
-        DmaDoneEvent(HDLcd *_obj)
-            : Event(), obj(*_obj), transSize(0) {}
-
-        /**
-         * Sets the size of this transaction.
-         *
-         * @param len size of the transaction in bytes
-         */
-        void setTransactionSize(size_t len) {
-            transSize = len;
-        }
-
-        /**
-         * Gets the size of this transaction.
-         *
-         * @return size of this transaction in bytes
-         */
-        size_t getTransactionSize() const {
-            return transSize;
-        }
-
-        void process() {
-            obj.dmaDone(this);
-        }
-
-        const std::string name() const {
-            return obj.name() + ".DmaDoneEvent";
-        }
-    };
-
-    /** Start time for frame buffer dma read */
-    Tick frameReadStartTime;
-
-    /** Starting address for the current frame */
-    Addr dmaStartAddr;
-
-    /** Next address the dma should read from */
-    Addr dmaCurAddr;
-
-    /** One byte past the address of the last byte the dma should read
-      * from */
-    Addr dmaMaxAddr;
-
-    /** Number of pending dma reads */
-    size_t dmaPendingNum;
-
-    /** Flag indicating whether current frame has underrun */
-    bool frameUnderrun;
-
-    /** HDLcd virtual display buffer */
-    std::vector<uint8_t> virtualDisplayBuffer;
-
-    /** Size of the pixel buffer */
-    size_t pixelBufferSize;
+    uint32_t readReg(Addr offset);
+    void writeReg(Addr offset, uint32_t value);
 
-    /** Index of the next pixel to render */
-    size_t pixelIndex;
-
-    /** Flag indicating whether video parameters need updating */
-    bool doUpdateParams;
+    PixelConverter pixelConverter() const;
+    DisplayTimings displayTimings() const;
 
-    /** Flag indicating whether a frame read / display is in progress */
-    bool frameUnderway;
+    void createDmaEngine();
 
-    /**
-     * Number of bytes in flight from DMA that have not reached the pixel
-     * buffer yet
-     */
-    uint32_t dmaBytesInFlight;
+    void cmdEnable();
+    void cmdDisable();
 
-    /**
-     * Gets the number of oustanding DMA transactions allowed on the bus at a
-     * time.
-     *
-     * @return gets the driver-specified number of outstanding DMA transactions
-     *         from the hdlcd controller that are allowed on the bus at a time
-     */
-    inline uint16_t maxOutstandingDma() const {
-        return bus_options.max_outstanding;
-    }
+    bool enabled() const { return command.enable; }
 
-    /**
-     * Gets the number of bytes free in the pixel buffer.
-     *
-     * @return number of bytes free in the internal pixel buffer
-     */
-    inline uint32_t bytesFreeInPixelBuffer() const {
-        return PIXEL_BUFFER_CAPACITY - (pixelBufferSize + dmaBytesInFlight);
-    }
+  public: // Pixel pump callbacks
+    bool pxlNext(Pixel &p);
+    void pxlVSyncBegin();
+    void pxlVSyncEnd();
+    void pxlUnderrun();
+    void pxlFrameDone();
 
+  protected: // Interrupt handling
     /**
-     * Gets the number of beats-per-burst for bus transactions.
+     * Assign new interrupt values and update interrupt signals
      *
-     * @return number of beats-per-burst per HDLcd DMA transaction
-     */
-    inline size_t dmaBurstLength() const {
-        assert(bus_options.burst_len <= MAX_BURST_LEN);
-        return bus_options.burst_len;
-    }
-
-    /**
-     * Gets the number of bytes per pixel.
+     * A new interrupt is scheduled signalled if the set of unmasked
+     * interrupts goes empty to non-empty. Conversely, if the set of
+     * unmasked interrupts goes from non-empty to empty, the interrupt
+     * signal is cleared.
      *
-     * @return bytes per pixel
+     * @param ints New <i>raw</i> interrupt status
+     * @param mask New interrupt mask
      */
-    inline size_t bytesPerPixel() const {
-        return pixel_format.bytes_per_pixel + 1;
-    }
+    void setInterrupts(uint32_t ints, uint32_t mask);
 
     /**
-     * Gets frame buffer width.
+     * Convenience function to update the interrupt mask
      *
-     * @return frame buffer width (pixels per line)
+     * @see setInterrupts
+     * @param mask New interrupt mask
      */
-    inline size_t width() const {
-        return fb_line_length / bytesPerPixel();
-    }
+    void intMask(uint32_t mask) { setInterrupts(int_rawstat, mask); }
 
     /**
-     * Gets frame buffer height.
+     * Convenience function to raise a new interrupt
      *
-     * @return frame buffer height (lines per panel)
+     * @see setInterrupts
+     * @param ints Set of interrupts to raise
      */
-    inline size_t height() const {
-        return fb_line_count.fb_line_count;
+    void intRaise(uint32_t ints) {
+        setInterrupts(int_rawstat | ints, int_mask);
     }
 
-    inline size_t area() const { return height() * width(); }
-
     /**
-     * Gets the total number of pixel clocks per display line.
+     * Convenience function to clear interrupts
      *
-     * @return number of pixel clocks per display line including porch delays
-     *         and horizontal sync time
+     * @see setInterrupts
+     * @param ints Set of interrupts to clear
      */
-    inline uint64_t PClksPerLine() const {
-        return h_back_porch.val + 1 +
-               h_data.val + 1 +
-               h_front_porch.val + 1 +
-               h_sync.val + 1;
+    void intClear(uint32_t ints) {
+        setInterrupts(int_rawstat & ~ints, int_mask);
     }
 
-    /** Send updated parameters to the vnc server */
-    void updateVideoParams(bool unserializing);
-
-    /** Generates an interrupt */
-    void generateInterrupt();
-
-    /** Start reading the next frame */
-    void startFrame();
+    /** Masked interrupt status register */
+    const uint32_t intStatus() const { return int_rawstat & int_mask; }
 
-    /** End of frame reached */
-    void endFrame();
-
-    /** Generate DMA read requests from frame buffer into pixel buffer */
-    void fillPixelBuffer();
+  protected: // Pixel output
+    class PixelPump : public BasePixelPump
+    {
+      public:
+        PixelPump(HDLcd &p, ClockDomain &pxl_clk, unsigned pixel_chunk)
+            : BasePixelPump(p, pxl_clk, pixel_chunk), parent(p) {}
 
-    /** DMA done event */
-    void dmaDone(DmaDoneEvent *event);
+        void dumpSettings();
 
-    /** Called when it is time to render a pixel */
-    void renderPixel();
+      protected:
+        bool nextPixel(Pixel &p) M5_ATTR_OVERRIDE { return parent.pxlNext(p); }
 
-    PixelConverter pixelConverter() const;
+        void onVSyncBegin() M5_ATTR_OVERRIDE { return parent.pxlVSyncBegin(); }
+        void onVSyncEnd() M5_ATTR_OVERRIDE { return parent.pxlVSyncEnd(); }
 
-    /** Start of frame event */
-    EventWrapper<HDLcd, &HDLcd::startFrame> startFrameEvent;
+        void onUnderrun(unsigned x, unsigned y) M5_ATTR_OVERRIDE {
+            parent.pxlUnderrun();
+        }
 
-    /** End of frame event */
-    EventWrapper<HDLcd, &HDLcd::endFrame> endFrameEvent;
+        void onFrameDone() M5_ATTR_OVERRIDE { parent.pxlFrameDone(); }
 
-    /** Pixel render event */
-    EventWrapper<HDLcd, &HDLcd::renderPixel> renderPixelEvent;
+      protected:
+        HDLcd &parent;
+    };
 
-    /** Fill fifo */
-    EventWrapper<HDLcd, &HDLcd::fillPixelBuffer> fillPixelBufferEvent;
+    /** Helper to write out bitmaps */
+    Bitmap bmp;
 
-    /** Wrapper to create an event out of the interrupt */
-    EventWrapper<HDLcd, &HDLcd::generateInterrupt> intEvent;
+    /** Picture of what the current frame buffer looks like */
+    std::ostream *pic;
 
-    /**@{*/
-    /**
-     * All pre-allocated DMA done events
-     *
-     * The HDLCD model preallocates maxOutstandingDma() number of
-     * DmaDoneEvents to avoid having to heap allocate every single
-     * event when it is needed. In order to keep track of which events
-     * are in flight and which are ready to be used, we use two
-     * different vectors. dmaDoneEventAll contains <i>all</i>
-     * DmaDoneEvents that the object may use, while dmaDoneEventFree
-     * contains a list of currently <i>unused</i> events. When an
-     * event needs to be scheduled, the last element of the
-     * dmaDoneEventFree is used and removed from the list. When an
-     * event fires, it is added to the end of the
-     * dmaEventFreeList. dmaDoneEventAll is never used except for in
-     * initialization and serialization.
-     */
-    std::vector<DmaDoneEvent> dmaDoneEventAll;
+    /** Cached pixel converter, set when the converter is enabled. */
+    PixelConverter conv;
 
-    /** Unused DMA done events that are ready to be scheduled */
-    std::vector<DmaDoneEvent *> dmaDoneEventFree;
-    /**@}*/
+    PixelPump pixelPump;
 
-    bool enableCapture;
+  protected: // DMA handling
+    class DmaEngine : public DmaReadFifo
+    {
+      public:
+        DmaEngine(HDLcd &_parent, size_t size,
+                  unsigned request_size, unsigned max_pending,
+                  size_t line_size, ssize_t line_pitch, unsigned num_lines);
 
-    const bool workaround_swap_rb;
+        void startFrame(Addr fb_base);
+        void abortFrame();
+        void dumpSettings();
 
-  public:
-    typedef HDLcdParams Params;
+        void serialize(CheckpointOut &cp) const M5_ATTR_OVERRIDE;
+        void unserialize(CheckpointIn &cp) M5_ATTR_OVERRIDE;
 
-    const Params *
-    params() const
-    {
-        return dynamic_cast<const Params *>(_params);
-    }
-    HDLcd(const Params *p);
-    ~HDLcd();
+      protected:
+        void onEndOfBlock() M5_ATTR_OVERRIDE;
+        void onIdle() M5_ATTR_OVERRIDE;
 
-    virtual Tick read(PacketPtr pkt);
-    virtual Tick write(PacketPtr pkt);
+        HDLcd &parent;
+        const size_t lineSize;
+        const ssize_t linePitch;
+        const unsigned numLines;
 
-    void serialize(CheckpointOut &cp) const M5_ATTR_OVERRIDE;
-    void unserialize(CheckpointIn &cp) M5_ATTR_OVERRIDE;
+        Addr nextLineAddr;
+        Addr frameEnd;
+    };
 
-    /**
-     * Determine the address ranges that this device responds to.
-     *
-     * @return a list of non-overlapping address ranges
-     */
-    AddrRangeList getAddrRanges() const;
+    std::unique_ptr<DmaEngine> dmaEngine;
 };
 
 #endif