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).

1 files changed, 140 insertions, 270 deletions

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