Self-Refresh Exit command handler improved. #39

efzulian · 2016-09-26T16:05:39Z

The first commit improves the code readability and adds documentation to all possible situations a SREX command may happen:

Self-refresh Exit Context 1 (tSREF >= tRFC):
The memory remained in self-refresh for a certain number of clock
cycles greater than a refresh cycle time (RFC). Consequently, the
initial auto-refresh accomplished.


 SREN                                #                SREX
 |                                   #                ^
 |                                   #                |
 |<------------------------- tSREF ----------...----->|
 |                                   #                |
 |      Initial Auto-Refresh         #                |
 v                                   #                |
 ------------------------------------#-------...-----------------> t
                                     #
  <------------- tRFC -------------->#
  <---- (tRFC - tRP) ----><-- tRP -->#
              |                |
              v                v
    sref_ref_act_cycles     sref_ref_pre_cycles


Summary:
sref_cycles_idd6 += tSREF – tRFC
sref_ref_act_cycles += tRFC - tRP
sref_ref_pre_cycles += tRP
spup_ref_act_cycles += 0
spup_ref_pre_cycles += 0



Self-refresh Exit Context 2A (tSREF < tRFC && tSREF >= tRFC - tRP):
The duration of self-refresh is equal or greater than the number
of active cycles needed by the initial auto-refresh.


 SREN                                            SREX
 |                                                ^         #
 |                                                |         #
 |<------------------ tSREF --------------------->|         #
 |                                                |         #
 |                                  Initial Auto-Refresh    #
 v                                                |         #
 -----------------------------------------------------------#--> t
                                                            #
  <------------------------ tRFC -------------------------->#
  <------------- (tRFC - tRP)--------------><----- tRP ---->#
          |                                 <-----><------->
          v                                  |         |
    sref_ref_act_cycles                      v         v
                            sref_ref_pre_cycles spup_ref_pre_cycles


Summary:
sref_cycles_idd6 += 0
sref_ref_act_cycles += tRFC - tRP
sref_ref_pre_cycles += tSREF – (tRFC – tRP)
spup_ref_act_cycles += 0
spup_ref_pre_cycles += tRP – sref_ref_pre_cycles



Self-refresh Exit Context 2B (tSREF < tRFC - tRP):
self-refresh duration is shorter than the number of active cycles
needed by the initial auto-refresh.


 SREN                             SREX
 |                                  ^                        #
 |                                  |                        #
 |<-------------- tSREF ----------->|                        #
 |                                  |                        #
 |                       Initial Auto-Refresh                #
 v                                  |                        #
 ------------------------------------------------------------#--> t
                                                             #
  <------------------------ tRFC --------------------------->#
  <-------------- (tRFC - tRP)-------------><------ tRP ---->#
  <--------------------------------><------><--------------->
              |                        |             |
              v                        v             v
    sref_ref_act_cycles    spup_ref_act_cycles spup_ref_pre_cycles


Summary:
sref_cycles_idd6 += 0
sref_ref_act_cycles += tSREF
sref_ref_pre_cycles += 0
spup_ref_act_cycles += (tRFC – tRP) - tSREF
spup_ref_pre_cycles += tRP

The second commit removes an overlap between "spup_ref_act_cycles" and "spup_ref_pre_cycles".

The variable "spup_ref_act_cycles" stores the number of active auto-refresh cycles during self-refresh exit.

The variable "spup_ref_pre_cycles" stores the number of precharged auto-refresh cycles during self-refresh exit.

tRP was counted for both "spup_ref_act_cycles" and "spup_ref_pre_cycles".

The variable "spup_ref_act_cycles" stores the number of active auto-refresh cycles during self-refresh exit. The variable "spup_ref_pre_cycles" stores the number of precharged auto-refresh cycles during self-refresh exit. tRP was counted for both "spup_ref_act_cycles" and "spup_ref_pre_cycles". This overlap was removed. Reference file for the "test_transaction_scheduler_with_self_refresh" updated accordingly.

coveralls · 2016-09-26T16:16:55Z

Coverage increased (+0.02%) to 83.563% when pulling 7249ab6 on efzulian:srefx_readability into b89db88 on ravenrd:master.

coveralls · 2016-09-26T16:16:55Z

Coverage increased (+0.02%) to 83.563% when pulling 7249ab6 on efzulian:srefx_readability into b89db88 on ravenrd:master.

efzulian · 2016-09-26T16:34:23Z

Dear Sven,
during a SREX command the variables "last_pre_cycle" (clock cycle of last precharge command when memory state changes to PRE) and "latest_pre_cycle" (last PRE cycle, not necessarily changing state to PRE) are updated.

Could you please help us to better understand how the variables update happen? Which clock cycle each of the variables should point after the update? Why?

Thanks!
Éder

Sv3n · 2016-09-28T19:34:51Z

last_pre_cycle is used to update the precycles variable, which essentially tracks the number of cycles the memory consumes IDD2N. This happens at SREFEN +tRFC or at SRX, which ever is later, so I would say:

last_pre_cycle = max(sref_cycle + memSpec.memTimingSpec.RFC, timestamp); // When a SRX command is being handled.

I am mostly basing this on this state machine image I built for the book chapter: http://imgur.com/a/txM7o

latest_pre_cycle updates idlecycles_pre, which is a sub-category of cycles in which IDD2N is consumed, i.e. they overlap with precycles, but have a different label in the final energy reporting. It looks like it is supposed to represent any cycle in which IDD2N is consumed, while NOT powering up from power down or self refresh. It should hence point to the cycle after the end of the <--RP--> arrows in your pictures, which is:

latest_pre_cycle = min(sref_cycle + memSpec.memTimingSpec.RFC, timestamp); // When a SRX command is being handled.

This looks a bit scary in context 1, since we are then effectively setting this cycle count to a time in the past, but assuming the controller doesn't issue commands between SREN and SREX, the regular accounting should still be correct as far as I can see.

efzulian · 2016-09-28T22:06:24Z

Hi Sven, thanks for the explanation. Nice state machine. :)
Since this part was not affected by this pull-request. I propose that we keep it untouched for now and limit this pull request to documentation and removal of the overlap between "spup_ref_act_cycles" and "spup_ref_pre_cycles".
Then later on we review all the points where last_pre_cycle and latest_pre_cycle are changed (not limited to the SREX, but other power down related commands).
What do you think?

myzinsky

I tried to mark approximately the suspicious code lines. As Éder proposes, we should discuss them maybe in an separate issue.

However, are you OK with removing the double counting of tRP?

myzinsky · 2016-09-29T12:28:30Z

src/CommandAnalysis.cc

+        // initial auto-refresh.
+        sref_cycles_idd6 += sref_duration - memSpec.memTimingSpec.RFC;
+
+        // IDD2N current is consumed when exiting the self-refresh state.
        if (memSpec.memArchSpec.dll == false) {
          spup_cycles     += memSpec.memTimingSpec.XS;
          latest_pre_cycle = max(timestamp, timestamp +


myzinsky · 2016-09-29T12:29:29Z

src/CommandAnalysis.cc

          last_pre_cycle       = timestamp + spup_pre;
+
          if (memSpec.memArchSpec.dll == false) {
            spup_cycles     += memSpec.memTimingSpec.XS - spup_pre;
            latest_pre_cycle = max(timestamp, timestamp +


myzinsky · 2016-09-29T12:29:42Z

src/CommandAnalysis.cc

@@ -481,18 +535,63 @@ void CommandAnalysis::evaluate(const MemorySpecification& memSpec,
                                 memSpec.memTimingSpec.XSDLL - memSpec.memTimingSpec.RCD


myzinsky · 2016-09-29T12:31:22Z

src/CommandAnalysis.cc

          spup_ref_act_cycles += spup_act;
          spup_ref_pre_cycles += memSpec.memTimingSpec.RP;
+
          last_pre_cycle       = timestamp + spup_act + memSpec.memTimingSpec.RP;
          if (memSpec.memArchSpec.dll == false) {
            spup_cycles     += memSpec.memTimingSpec.XS - spup_act -


efzulian · 2016-09-29T19:11:43Z

src/CommandAnalysis.cc

-          // FIXME: Here should be ((memSpec.memTimingSpec.RFC - memSpec.memTimingSpec.RP) - sref_duration) to avoid overlapping !!
-          int64_t spup_act = memSpec.memTimingSpec.RFC - sref_duration;
+          int64_t spup_act = (memSpec.memTimingSpec.RFC -
+                              memSpec.memTimingSpec.RP) - sref_duration;



Only this change affects the output (the value assigned to spup_act has changed). It corresponds to the second commit.

The first commit was intended to bring minor improvements to the documentation.

Sv3n · 2016-09-29T19:51:09Z

src/CommandAnalysis.cc

+      int64_t sref_duration = timestamp - sref_cycle;
+
+      // Negative or zero duration should never happen.
+      assert(sref_duration > 0);


We have never punished users that generate a broken command trace with a hard assert. Perhaps it is better to print a warning here? We could consider adding a flag in a future version that turns warning prints into asserts.

Personally, I don't see the assertion as a punishment, but as a helper. As far I can see, people are only interested in valid traces. If this assumption holds, the assertion will ensure that the self-refresh duration is greater than zero (it may be possible that the user fixes the trace for his own benefit). The same principle applies to the second assertion inserted.

However, if you have a strong preference for warnings instead of assertions just let me know.

Other changes: - "warnings.trace" updated to explore new warning conditions during "test_broken_trace". - Error output of "TestLibDRAMPower" redirected to avoid unnecessary polution in "make test" output. - Some comments improved to use the code nomenclature: SREFEN --> SREN and SREFX --> SREX.

coveralls · 2016-09-30T15:00:56Z

Coverage increased (+0.2%) to 83.715% when pulling 3e430c6 on efzulian:srefx_readability into 9badc1d on ravenrd:master.

efzulian · 2016-09-30T15:17:12Z

Hi Sven,
you're right, it seems that the best approach is to print warnings. In a future version we revisit all warnings, and evaluate all of them in order to implement a common treatment (possibly using a flag as you suggested).

Sv3n · 2016-09-30T18:39:37Z

Thanks for changing it. I think we should give users a chance to potentially fix their fault command traces / memory controllers before we introduce hard-exits.

efzulian added 2 commits September 26, 2016 16:23

Self-Refresh Exit command handler improved and documented

ae41323

myzinsky reviewed Sep 29, 2016

View reviewed changes

efzulian commented Sep 29, 2016

View reviewed changes

Sv3n reviewed Sep 29, 2016

View reviewed changes

efzulian added 2 commits September 30, 2016 16:42

Merge remote-tracking branch 'upstream/master' into srefx_readability

3e430c6

Sv3n merged commit 8fa166d into tukl-msd:master Sep 30, 2016

efzulian deleted the srefx_readability branch September 30, 2016 22:21

efzulian mentioned this pull request Oct 11, 2016

Review clock cycle counters when exiting self-refresh. #40

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Self-Refresh Exit command handler improved. #39

Self-Refresh Exit command handler improved. #39

efzulian commented Sep 26, 2016 •

edited

coveralls commented Sep 26, 2016

coveralls commented Sep 26, 2016

efzulian commented Sep 26, 2016 •

edited

Sv3n commented Sep 28, 2016

efzulian commented Sep 28, 2016

myzinsky left a comment

myzinsky Sep 29, 2016

myzinsky Sep 29, 2016

myzinsky Sep 29, 2016

myzinsky Sep 29, 2016

efzulian Sep 29, 2016

Sv3n Sep 29, 2016

efzulian Sep 29, 2016

coveralls commented Sep 30, 2016 •

edited

efzulian commented Sep 30, 2016 •

edited

Sv3n commented Sep 30, 2016

		@@ -481,18 +535,63 @@ void CommandAnalysis::evaluate(const MemorySpecification& memSpec,
		memSpec.memTimingSpec.XSDLL - memSpec.memTimingSpec.RCD

Self-Refresh Exit command handler improved. #39

Self-Refresh Exit command handler improved. #39

Conversation

efzulian commented Sep 26, 2016 • edited

coveralls commented Sep 26, 2016

coveralls commented Sep 26, 2016

efzulian commented Sep 26, 2016 • edited

Sv3n commented Sep 28, 2016

efzulian commented Sep 28, 2016

myzinsky left a comment

Choose a reason for hiding this comment

myzinsky Sep 29, 2016

Choose a reason for hiding this comment

myzinsky Sep 29, 2016

Choose a reason for hiding this comment

myzinsky Sep 29, 2016

Choose a reason for hiding this comment

myzinsky Sep 29, 2016

Choose a reason for hiding this comment

efzulian Sep 29, 2016

Choose a reason for hiding this comment

Sv3n Sep 29, 2016

Choose a reason for hiding this comment

efzulian Sep 29, 2016

Choose a reason for hiding this comment

coveralls commented Sep 30, 2016 • edited

efzulian commented Sep 30, 2016 • edited

Sv3n commented Sep 30, 2016

efzulian commented Sep 26, 2016 •

edited

efzulian commented Sep 26, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited

efzulian commented Sep 30, 2016 •

edited