From 2c1052cd4d1bef2965e449ad08130acec9b2ce38 Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Wed, 22 Aug 2012 11:40:01 -0400 Subject: [PATCH] DMA: Refactor the DMA device and align timing and atomic This patch does a bunch of house-keeping updates on the DMA, including indentation, and formatting, but most importantly breaks out the response handling such that it can be shared between the atomic and timing modes. It also removes a potential bug caused by the atomic handling of responses only deleting the allocated request (pkt->req) once the DMA action completes instead of doing so for every packet. Before this patch, the handling of responses was near identical for atomic and timing, but the code was simply duplicated. With this patch, the handleResp method deals with the responses in both cases. There are further updates to make after removing the NACKs, but that will be part of a separate follow-up patch. This patch does not change the behaviour of any regression. --- src/dev/dma_device.cc | 175 +++++++++++++++++++----------------------- src/dev/dma_device.hh | 39 ++++++---- 2 files changed, 104 insertions(+), 110 deletions(-) diff --git a/src/dev/dma_device.cc b/src/dev/dma_device.cc index 80253129f..c64694315 100644 --- a/src/dev/dma_device.cc +++ b/src/dev/dma_device.cc @@ -39,6 +39,7 @@ * * Authors: Ali Saidi * Nathan Binkert + * Andreas Hansson */ #include "base/chunk_generator.hh" @@ -54,45 +55,62 @@ DmaPort::DmaPort(MemObject *dev, System *s) inRetry(false) { } +void +DmaPort::handleResp(PacketPtr pkt, Tick delay) +{ + // should always see a response with a sender state + assert(pkt->isResponse()); + + // get the DMA sender state + DmaReqState *state = dynamic_cast(pkt->senderState); + assert(state); + + DPRINTF(DMA, "Received response %s for addr: %#x size: %d nb: %d," \ + " tot: %d sched %d\n", + pkt->cmdString(), pkt->getAddr(), pkt->req->getSize(), + state->numBytes, state->totBytes, + state->completionEvent ? + state->completionEvent->scheduled() : 0); + + assert(pendingCount != 0); + pendingCount--; + + // update the number of bytes received based on the request rather + // than the packet as the latter could be rounded up to line sizes + state->numBytes += pkt->req->getSize(); + assert(state->totBytes >= state->numBytes); + + // if we have reached the total number of bytes for this DMA + // request, then signal the completion and delete the sate + if (state->totBytes == state->numBytes) { + if (state->completionEvent) { + delay += state->delay; + if (delay) + device->schedule(state->completionEvent, curTick() + delay); + else + state->completionEvent->process(); + } + delete state; + } + + // delete the request that we created and also the packet + delete pkt->req; + delete pkt; + + // we might be drained at this point, if so signal the drain event + if (pendingCount == 0 && drainEvent) { + drainEvent->process(); + drainEvent = NULL; + } +} + bool DmaPort::recvTimingResp(PacketPtr pkt) { - if (pkt->senderState) { - DmaReqState *state; + // We shouldn't ever get a block in ownership state + assert(!(pkt->memInhibitAsserted() && !pkt->sharedAsserted())); - DPRINTF(DMA, "Received response %s addr %#x size %#x\n", - pkt->cmdString(), pkt->getAddr(), pkt->req->getSize()); - state = dynamic_cast(pkt->senderState); - pendingCount--; - - assert(pendingCount >= 0); - assert(state); - - // We shouldn't ever get a block in ownership state - assert(!(pkt->memInhibitAsserted() && !pkt->sharedAsserted())); - - state->numBytes += pkt->req->getSize(); - assert(state->totBytes >= state->numBytes); - if (state->totBytes == state->numBytes) { - if (state->completionEvent) { - if (state->delay) - device->schedule(state->completionEvent, - curTick() + state->delay); - else - state->completionEvent->process(); - } - delete state; - } - delete pkt->req; - delete pkt; - - if (pendingCount == 0 && transmitList.empty() && drainEvent) { - drainEvent->process(); - drainEvent = NULL; - } - } else { - panic("Got packet without sender state... huh?\n"); - } + handleResp(pkt); return true; } @@ -112,8 +130,7 @@ DmaDevice::init() unsigned int DmaDevice::drain(Event *de) { - unsigned int count; - count = pioPort.drain(de) + dmaPort.drain(de); + unsigned int count = pioPort.drain(de) + dmaPort.drain(de); if (count) changeState(Draining); else @@ -124,7 +141,7 @@ DmaDevice::drain(Event *de) unsigned int DmaPort::drain(Event *de) { - if (transmitList.empty() && pendingCount == 0) + if (pendingCount == 0) return 0; drainEvent = de; DPRINTF(Drain, "DmaPort not drained\n"); @@ -159,46 +176,46 @@ void DmaPort::dmaAction(Packet::Command cmd, Addr addr, int size, Event *event, uint8_t *data, Tick delay, Request::Flags flag) { + // one DMA request sender state for every action, that is then + // split into many requests and packets based on the block size, + // i.e. cache line size DmaReqState *reqState = new DmaReqState(event, size, delay); - DPRINTF(DMA, "Starting DMA for addr: %#x size: %d sched: %d\n", addr, size, - event ? event->scheduled() : -1 ); + event ? event->scheduled() : -1); for (ChunkGenerator gen(addr, size, peerBlockSize()); !gen.done(); gen.next()) { - Request *req = new Request(gen.addr(), gen.size(), flag, masterId); - PacketPtr pkt = new Packet(req, cmd); + Request *req = new Request(gen.addr(), gen.size(), flag, masterId); + PacketPtr pkt = new Packet(req, cmd); - // Increment the data pointer on a write - if (data) - pkt->dataStatic(data + gen.complete()); + // Increment the data pointer on a write + if (data) + pkt->dataStatic(data + gen.complete()); - pkt->senderState = reqState; + pkt->senderState = reqState; - assert(pendingCount >= 0); - pendingCount++; - DPRINTF(DMA, "--Queuing DMA for addr: %#x size: %d\n", gen.addr(), - gen.size()); - queueDma(pkt); + DPRINTF(DMA, "--Queuing DMA for addr: %#x size: %d\n", gen.addr(), + gen.size()); + queueDma(pkt); } - } void -DmaPort::queueDma(PacketPtr pkt, bool front) +DmaPort::queueDma(PacketPtr pkt) { + transmitList.push_back(pkt); + + // remember that we have another packet pending, this will only be + // decremented once a response comes back + pendingCount++; - if (front) - transmitList.push_front(pkt); - else - transmitList.push_back(pkt); sendDma(); } void DmaPort::sendDma() { - // some kind of selction between access methods + // some kind of selcetion between access methods // more work is going to have to be done to make // switching actually work assert(transmitList.size()); @@ -228,45 +245,13 @@ DmaPort::sendDma() } else if (state == Enums::atomic) { transmitList.pop_front(); - Tick lat; - DPRINTF(DMA, "--Sending DMA for addr: %#x size: %d\n", + DPRINTF(DMA, "Sending DMA for addr: %#x size: %d\n", pkt->req->getPaddr(), pkt->req->getSize()); - lat = sendAtomic(pkt); - assert(pkt->senderState); - DmaReqState *state = dynamic_cast(pkt->senderState); - assert(state); - state->numBytes += pkt->req->getSize(); + Tick lat = sendAtomic(pkt); - DPRINTF(DMA, "--Received response for DMA for addr: %#x size: %d nb: %d, tot: %d sched %d\n", - pkt->req->getPaddr(), pkt->req->getSize(), state->numBytes, - state->totBytes, - state->completionEvent ? state->completionEvent->scheduled() : 0 ); - - if (state->totBytes == state->numBytes) { - if (state->completionEvent) { - assert(!state->completionEvent->scheduled()); - device->schedule(state->completionEvent, - curTick() + lat + state->delay); - } - delete state; - delete pkt->req; - } - pendingCount--; - assert(pendingCount >= 0); - delete pkt; - - if (pendingCount == 0 && transmitList.empty() && drainEvent) { - DPRINTF(Drain, "DmaPort done draining, processing drain event\n"); - drainEvent->process(); - drainEvent = NULL; - } - - } else - panic("Unknown memory command state."); -} - -DmaDevice::~DmaDevice() -{ + handleResp(pkt, lat); + } else + panic("Unknown memory mode."); } MasterPort & diff --git a/src/dev/dma_device.hh b/src/dev/dma_device.hh index 691a21749..48c3f9c61 100644 --- a/src/dev/dma_device.hh +++ b/src/dev/dma_device.hh @@ -44,6 +44,8 @@ #ifndef __DEV_DMA_DEVICE_HH__ #define __DEV_DMA_DEVICE_HH__ +#include + #include "dev/io_device.hh" #include "params/DmaDevice.hh" @@ -71,7 +73,9 @@ class DmaPort : public MasterPort }; MemObject *device; - std::list transmitList; + + /** Use a deque as we never to any insertion or removal in the middle */ + std::deque transmitList; /** The system that device/port are in. This is used to select which mode * we are currently operating in. */ @@ -81,21 +85,32 @@ class DmaPort : public MasterPort MasterID masterId; /** Number of outstanding packets the dma port has. */ - int pendingCount; + uint32_t pendingCount; /** If we need to drain, keep the drain event around until we're done * here.*/ Event *drainEvent; - /** If the port is currently waiting for a retry before it can send whatever - * it is that it's sending. */ + /** If the port is currently waiting for a retry before it can + * send whatever it is that it's sending. */ bool inRetry; - virtual bool recvTimingResp(PacketPtr pkt); + /** + * Handle a response packet by updating the corresponding DMA + * request state to reflect the bytes received, and also update + * the pending request counter. If the DMA request that this + * packet is part of is complete, then signal the completion event + * if present, potentially with a delay added to it. + * + * @param pkt Response packet to handler + * @param delay Additional delay for scheduling the completion event + */ + void handleResp(PacketPtr pkt, Tick delay = 0); - virtual void recvRetry() ; + bool recvTimingResp(PacketPtr pkt); + void recvRetry() ; - void queueDma(PacketPtr pkt, bool front = false); + void queueDma(PacketPtr pkt); void sendDma(); public: @@ -105,7 +120,7 @@ class DmaPort : public MasterPort void dmaAction(Packet::Command cmd, Addr addr, int size, Event *event, uint8_t *data, Tick delay, Request::Flags flag = 0); - bool dmaPending() { return pendingCount > 0; } + bool dmaPending() const { return pendingCount > 0; } unsigned cacheBlockSize() const { return peerBlockSize(); } unsigned int drain(Event *de); @@ -119,13 +134,7 @@ class DmaDevice : public PioDevice public: typedef DmaDeviceParams Params; DmaDevice(const Params *p); - virtual ~DmaDevice(); - - const Params * - params() const - { - return dynamic_cast(_params); - } + virtual ~DmaDevice() { } void dmaWrite(Addr addr, int size, Event *event, uint8_t *data, Tick delay = 0)