summaryrefslogtreecommitdiff
path: root/src/dev/arm
diff options
context:
space:
mode:
authorAndreas Sandberg <Andreas.Sandberg@ARM.com>2013-01-07 13:05:37 -0500
committerAndreas Sandberg <Andreas.Sandberg@ARM.com>2013-01-07 13:05:37 -0500
commit63f1d0516d8a4e5a688cc3000b5a3597518d3dc7 (patch)
treea1a33ec62895dd6f8fa2de234a6186112e33b40c /src/dev/arm
parentfffdc6a45019639cd8f899fa81eeb732db3e6f8c (diff)
downloadgem5-63f1d0516d8a4e5a688cc3000b5a3597518d3dc7.tar.xz
arm: Fix DMA event handling bug in the PL111 model
The PL111 model currently maintains a list of pre-allocated DmaDoneEvents to prevent unnecessary heap allocations. This list effectively works like a stack where the top element is the latest scheduled event. When an event triggers, the top pointer is moved down the stack. This obviously breaks since events usually retire from the bottom (events don't necessarily have to retire in order), which triggers the following assertion: gem5.debug: build/ARM/dev/arm/pl111.cc:460: void Pl111::fillFifo(): \ Assertion `!dmaDoneEvent[dmaPendingNum-1].scheduled()' failed. This changeset adds a vector listing the currently unused events. This vector acts like a stack where the an element is popped off the stack when a new event is needed an pushed on the stack when they trigger.
Diffstat (limited to 'src/dev/arm')
-rw-r--r--src/dev/arm/pl111.cc24
-rw-r--r--src/dev/arm/pl111.hh51
2 files changed, 66 insertions, 9 deletions
diff --git a/src/dev/arm/pl111.cc b/src/dev/arm/pl111.cc
index 2973cda27..cd1f00272 100644
--- a/src/dev/arm/pl111.cc
+++ b/src/dev/arm/pl111.cc
@@ -68,7 +68,9 @@ Pl111::Pl111(const Params *p)
vnc(p->vnc), bmp(NULL), width(LcdMaxWidth), height(LcdMaxHeight),
bytesPerPixel(4), startTime(0), startAddr(0), maxAddr(0), curAddr(0),
waterMark(0), dmaPendingNum(0), readEvent(this), fillFifoEvent(this),
- dmaDoneEvent(maxOutstandingDma, this), intEvent(this)
+ dmaDoneEventAll(maxOutstandingDma, this),
+ dmaDoneEventFree(maxOutstandingDma),
+ intEvent(this)
{
pioSize = 0xFFFF;
@@ -81,6 +83,9 @@ Pl111::Pl111(const Params *p)
memset(cursorImage, 0, sizeof(cursorImage));
memset(dmaBuffer, 0, buffer_size);
+ for (int i = 0; i < maxOutstandingDma; ++i)
+ dmaDoneEventFree[i] = &dmaDoneEventAll[i];
+
if (vnc)
vnc->setFramebufferAddr(dmaBuffer);
}
@@ -458,14 +463,17 @@ Pl111::fillFifo()
// due to assertion in scheduling state
++dmaPendingNum;
- assert(!dmaDoneEvent[dmaPendingNum-1].scheduled());
+ assert(!dmaDoneEventFree.empty());
+ DmaDoneEvent *event(dmaDoneEventFree.back());
+ dmaDoneEventFree.pop_back();
+ 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
dmaPort.dmaAction(MemCmd::ReadReq, curAddr + startAddr, dmaSize,
- &dmaDoneEvent[dmaPendingNum-1], curAddr + dmaBuffer,
+ event, curAddr + dmaBuffer,
0, Request::UNCACHEABLE);
curAddr += dmaSize;
}
@@ -599,8 +607,8 @@ Pl111::serialize(std::ostream &os)
vector<Tick> dma_done_event_tick;
dma_done_event_tick.resize(maxOutstandingDma);
for (int x = 0; x < maxOutstandingDma; x++) {
- dma_done_event_tick[x] = dmaDoneEvent[x].scheduled() ?
- dmaDoneEvent[x].when() : 0;
+ dma_done_event_tick[x] = dmaDoneEventAll[x].scheduled() ?
+ dmaDoneEventAll[x].when() : 0;
}
arrayParamOut(os, "dma_done_event_tick", dma_done_event_tick);
}
@@ -701,10 +709,14 @@ Pl111::unserialize(Checkpoint *cp, const std::string &section)
vector<Tick> dma_done_event_tick;
dma_done_event_tick.resize(maxOutstandingDma);
arrayParamIn(cp, section, "dma_done_event_tick", dma_done_event_tick);
+ dmaDoneEventFree.clear();
for (int x = 0; x < maxOutstandingDma; x++) {
if (dma_done_event_tick[x])
- schedule(dmaDoneEvent[x], dma_done_event_tick[x]);
+ schedule(dmaDoneEventAll[x], dma_done_event_tick[x]);
+ else
+ dmaDoneEventFree.push_back(&dmaDoneEventAll[x]);
}
+ assert(maxOutstandingDma - dmaDoneEventFree.size() == dmaPendingNum);
if (lcdControl.lcdpwr) {
updateVideoParams();
diff --git a/src/dev/arm/pl111.hh b/src/dev/arm/pl111.hh
index 2388e1c7a..2245d8124 100644
--- a/src/dev/arm/pl111.hh
+++ b/src/dev/arm/pl111.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010 ARM Limited
+ * Copyright (c) 2010-2012 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -161,6 +161,31 @@ class Pl111: public AmbaDmaDevice
Bitfield<16> watermark;
EndBitUnion(ControlReg)
+ /**
+ * Event wrapper for dmaDone()
+ *
+ * This event calls pushes its this pointer onto the freeDoneEvent
+ * vector and calls dmaDone() when triggered.
+ */
+ class DmaDoneEvent : public Event
+ {
+ private:
+ Pl111 &obj;
+
+ public:
+ DmaDoneEvent(Pl111 *_obj)
+ : Event(), obj(*_obj) {}
+
+ void process() {
+ obj.dmaDoneEventFree.push_back(this);
+ obj.dmaDone();
+ }
+
+ const std::string name() const {
+ return obj.name() + ".DmaDoneEvent";
+ }
+ };
+
/** Horizontal axis panel control register */
TimingReg0 lcdTiming0;
@@ -296,8 +321,28 @@ class Pl111: public AmbaDmaDevice
/** Fill fifo */
EventWrapper<Pl111, &Pl111::fillFifo> fillFifoEvent;
- /** DMA done event */
- std::vector<EventWrapper<Pl111, &Pl111::dmaDone> > dmaDoneEvent;
+ /**@{*/
+ /**
+ * All pre-allocated DMA done events
+ *
+ * The PL111 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;
+
+ /** Unused DMA done events that are ready to be scheduled */
+ std::vector<DmaDoneEvent *> dmaDoneEventFree;
+ /**@}*/
/** Wrapper to create an event out of the interrupt */
EventWrapper<Pl111, &Pl111::generateInterrupt> intEvent;