merged with 62e1504b9c64

3 files changed, 628 insertions, 918 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..2c402b00a 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,187 @@
 #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::regStats()
+{
+    using namespace Stats;
+
+    stats.underruns
+        .name(name() + ".underruns")
+        .desc("number of buffer underruns")
+        .flags(nozero)
+        ;
+}
+
+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 +235,428 @@ 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;
+}
+
+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);
+    }
+}
 
-    switch (daddr) {
+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");
+    ++stats.underruns;
+    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..cb47b8522 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,41 @@
 #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 regStats() M5_ATTR_OVERRIDE;
+
+    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 +142,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;
-
-    /** 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 constexpr size_t MAX_BURST_LEN = 16;
 
-    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 +171,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 +229,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 +253,141 @@ 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;
+    uint32_t readReg(Addr offset);
+    void writeReg(Addr offset, uint32_t value);
 
-    /** Size of the pixel buffer */
-    size_t pixelBufferSize;
-
-    /** 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();
+    /** Masked interrupt status register */
+    const uint32_t intStatus() const { return int_rawstat & int_mask; }
 
-    /** Start reading the next frame */
-    void startFrame();
-
-    /** End of frame reached */
-    void endFrame();
+  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) {}
 
-    /** Generate DMA read requests from frame buffer into pixel buffer */
-    void fillPixelBuffer();
+        void dumpSettings();
 
-    /** DMA done event */
-    void dmaDone(DmaDoneEvent *event);
+      protected:
+        bool nextPixel(Pixel &p) M5_ATTR_OVERRIDE { return parent.pxlNext(p); }
 
-    /** Called when it is time to render a pixel */
-    void renderPixel();
+        void onVSyncBegin() M5_ATTR_OVERRIDE { return parent.pxlVSyncBegin(); }
+        void onVSyncEnd() M5_ATTR_OVERRIDE { return parent.pxlVSyncEnd(); }
 
-    PixelConverter pixelConverter() const;
+        void onUnderrun(unsigned x, unsigned y) M5_ATTR_OVERRIDE {
+            parent.pxlUnderrun();
+        }
 
-    /** Start of frame event */
-    EventWrapper<HDLcd, &HDLcd::startFrame> startFrameEvent;
+        void onFrameDone() M5_ATTR_OVERRIDE { parent.pxlFrameDone(); }
 
-    /** End of frame event */
-    EventWrapper<HDLcd, &HDLcd::endFrame> endFrameEvent;
+      protected:
+        HDLcd &parent;
+    };
 
-    /** Pixel render event */
-    EventWrapper<HDLcd, &HDLcd::renderPixel> renderPixelEvent;
+    /** Helper to write out bitmaps */
+    Bitmap bmp;
 
-    /** Fill fifo */
-    EventWrapper<HDLcd, &HDLcd::fillPixelBuffer> fillPixelBufferEvent;
+    /** Picture of what the current frame buffer looks like */
+    std::ostream *pic;
 
-    /** Wrapper to create an event out of the interrupt */
-    EventWrapper<HDLcd, &HDLcd::generateInterrupt> intEvent;
+    /** Cached pixel converter, set when the converter is enabled. */
+    PixelConverter conv;
 
-    /**@{*/
-    /**
-     * 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;
+    PixelPump pixelPump;
 
-    /** Unused DMA done events that are ready to be scheduled */
-    std::vector<DmaDoneEvent *> dmaDoneEventFree;
-    /**@}*/
+  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);
 
-    bool enableCapture;
+        void startFrame(Addr fb_base);
+        void abortFrame();
+        void dumpSettings();
 
-    const bool workaround_swap_rb;
+        void serialize(CheckpointOut &cp) const M5_ATTR_OVERRIDE;
+        void unserialize(CheckpointIn &cp) M5_ATTR_OVERRIDE;
 
-  public:
-    typedef HDLcdParams Params;
+      protected:
+        void onEndOfBlock() M5_ATTR_OVERRIDE;
+        void onIdle() M5_ATTR_OVERRIDE;
 
-    const Params *
-    params() const
-    {
-        return dynamic_cast<const Params *>(_params);
-    }
-    HDLcd(const Params *p);
-    ~HDLcd();
+        HDLcd &parent;
+        const size_t lineSize;
+        const ssize_t linePitch;
+        const unsigned numLines;
 
-    virtual Tick read(PacketPtr pkt);
-    virtual Tick write(PacketPtr pkt);
+        Addr nextLineAddr;
+        Addr frameEnd;
+    };
 
-    void serialize(CheckpointOut &cp) const M5_ATTR_OVERRIDE;
-    void unserialize(CheckpointIn &cp) M5_ATTR_OVERRIDE;
+    std::unique_ptr<DmaEngine> dmaEngine;
 
-    /**
-     * Determine the address ranges that this device responds to.
-     *
-     * @return a list of non-overlapping address ranges
-     */
-    AddrRangeList getAddrRanges() const;
+  protected: // Statistics
+    struct {
+        Stats::Scalar underruns;
+    } stats;
 };
 
 #endif