Hello Wendy Elsasser,

I'd like you to do a code review. Please visit

    https://gem5-review.googlesource.com/c/public/gem5/+/30056

to review the following change.


Change subject: mem: Use beats_per_clock as the DDR data rate for DRAMPower
......................................................................

mem: Use beats_per_clock as the DDR data rate for DRAMPower

The data rate is used by the drampower lib to estimate the power
consumption of the DRAM Core. Previously, we used the formula:

burst_cycles = divCeil(p->tBURST_MAX, p->tCK);
data_rate = p->burst_length / burst_cycles;

to derive the date_rate. However, under certain configurations this
formula computes the wrong result due to rounding errors. This patch
simplifies the way we derive the date_rate by passing the value of the
DRAM parameter beats_per_clock.

Change-Id: Ic8cd35bb4641d9c0a704675d2672a6fe4f4ec13e
Signed-off-by: Nikos Nikoleris <nikos.nikole...@arm.com>
Reviewed-by: Wendy Elsasser <wendy.elsas...@arm.com>
---
M src/mem/drampower.cc
M src/mem/drampower.hh
2 files changed, 1 insertion(+), 17 deletions(-)



diff --git a/src/mem/drampower.cc b/src/mem/drampower.cc
index f506928..13551a0 100644
--- a/src/mem/drampower.cc
+++ b/src/mem/drampower.cc
@@ -53,7 +53,7 @@
     archSpec.nbrOfBanks = p->banks_per_rank;
     // One DRAMPower instance per rank, hence set this to 1
     archSpec.nbrOfRanks = 1;
-    archSpec.dataRate = getDataRate(p);
+    archSpec.dataRate = p->beats_per_clock;
     // For now we can ignore the number of columns and rows as they
     // are not used in the power calculation.
     archSpec.nbrOfColumns = 0;
@@ -146,14 +146,3 @@
 {
     return p->VDD2 == 0 ? false : true;
 }
-
-uint8_t
-DRAMPower::getDataRate(const DRAMCtrlParams* p)
-{
-    uint32_t burst_cycles = divCeil(p->tBURST_MAX, p->tCK);
-    uint8_t data_rate = p->burst_length / burst_cycles;
-    // 4 for GDDR5
- if (data_rate != 1 && data_rate != 2 && data_rate != 4 && data_rate != 8)
-        fatal("Got unexpected data rate %d, should be 1 or 2 or 4 or 8\n");
-    return data_rate;
-}
diff --git a/src/mem/drampower.hh b/src/mem/drampower.hh
index ed47476..da24bca 100644
--- a/src/mem/drampower.hh
+++ b/src/mem/drampower.hh
@@ -74,11 +74,6 @@
     static Data::MemPowerSpec getPowerParams(const DRAMCtrlParams* p);

     /**
-     * Determine data rate, either one or two.
-     */
-    static uint8_t getDataRate(const DRAMCtrlParams* p);
-
-    /**
      * Determine if DRAM has two voltage domains (or one)
      */
     static bool hasTwoVDD(const DRAMCtrlParams* p);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30056
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ic8cd35bb4641d9c0a704675d2672a6fe4f4ec13e
Gerrit-Change-Number: 30056
Gerrit-PatchSet: 1
Gerrit-Owner: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: Wendy Elsasser <wendy.elsas...@arm.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to