Now I understand your other question "Also, do you not think it is wise to use the Param.MemorySize here and specify it in bytes of kb?" I think memory size is a function rowbuffer size and number of rows per DRAM array and not the other way around.
Amin On Tue, Jun 25, 2013 at 9:36 AM, Amin Farmahini <[email protected]> wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1927/ > > On June 25th, 2013, 10:12 a.m. UTC, *Andreas Hansson* wrote: > > Overall this looks like a great step in the right direction. One thing that > struck me is the complexity of the iteration to split the packets. Why make > it so complicated? Would we not simply change decode packet to take a > container as an input parameter and push back as many packets as needed? This > should also help remove any assumptions on alignment. What do you think? > > I read your comment multiple times to grasp it, but still not sure I fully > understand it. > > "change decode packet to take a container as an input parameter and push back > as many packets as needed". before pushing each split packet, I make sure > this split packet is not in the write queue. If it is in the write queue, we > don't push it back. If we push all split packets at once, there is no way to > check the write queue. Please, note that a packet could be split into > multiple split burst-size packets in which some burst-size packets could be > in the write queue and some not. > > Overall, I am open to discuss it further and willing to make it more simple, > if possible. > > > On June 25th, 2013, 10:12 a.m. UTC, *Andreas Hansson* wrote: > > > src/mem/SimpleDRAM.py<http://reviews.gem5.org/r/1927/diff/2/?file=36306#file36306line121> > (Diff > revision 2) > > def makeMultiChannel(cls, nbr_mem_ctrls, mem_start_addr, mem_size, > > 121 > > lines_per_rowbuffer = Param.Unsigned("Row buffer size in cache lines") > > 121 > > device_bus_width = Param.Unsigned("data bus width in bits for each DRAM "\ > > Do we need it per device? Why not combined? > > I believe this way is more clear. we use devicesPerRank to calculate the > combined bus width. (devicesPerRank * deviceBusWidth) > The devicesPerRank parameter helps us find out how many devices we have. As > in our current configurations, some DRAMs have 8 devices, and some only 1. It > is better to distinguish between them and make it more clear how many devices > we have. > > > On June 25th, 2013, 10:12 a.m. UTC, *Andreas Hansson* wrote: > > > src/mem/SimpleDRAM.py<http://reviews.gem5.org/r/1927/diff/2/?file=36306#file36306line124> > (Diff > revision 2) > > def makeMultiChannel(cls, nbr_mem_ctrls, mem_start_addr, mem_size, > > 124 > > device_rowbuffer_size = Param.Unsigned("Page (row buffer) size per "\ > > Same as above, why per device and not per channel? > > Also, do you not think it is wise to use the Param.MemorySize here and > specify it in bytes of kb? > > similarly, we use devicesPerRank to calculate the total row buffer size => > (simple_dram.cc:108) rowBufferSize = devicesPerRank * deviceRowBufferSize; > The reason is clarity again. > > For your other question, so do you mean we want to have a fixed memory size? > > > On June 25th, 2013, 10:12 a.m. UTC, *Andreas Hansson* wrote: > > > src/mem/simple_dram.hh<http://reviews.gem5.org/r/1927/diff/2/?file=36307#file36307line435> > (Diff > revision 2) > > class SimpleDRAM : public AbstractMemory > > 435 > > uint32_t burstSize; > > I'd suggest to make all these const > > Please note that burst_length and burst_size are different. burst_length is > defined const, but burst_size is not const because burst_size is initialized > in the body just like rowsPerBank. > > > - Amin > > On June 24th, 2013, 6:01 a.m. UTC, Amin Farmahini wrote: > Review request for Default. > By Amin Farmahini. > > *Updated June 24, 2013, 6:01 a.m.* > *Repository: * gem5 > Description > > This patch gets rid of bytesPerCacheLine parameter and makes the DRAM > configuration separate from cache line size. > Instead of bytesPerCacheLine, I define a parameter for DRAM called > burst_length. The burst_length parameter shows the length of a DRAM device > burst in bits. > Also, I replace lines_per_rowbuffer with device_rowbuffer_size to improve > code portablity. > > Updates: > - a burst length in beats for each memory type. > - an interface width for each memory type. > - the memory controller model is extended to reason about "system" packets vs > "dram" packets and assemble the responses properly. It means that system > packets larger than a full burst are split into multiple dram packets. > > Testing > > None > > Diffs > > - src/mem/simple_dram.cc (UNKNOWN) > - src/mem/simple_dram.hh (UNKNOWN) > - src/mem/SimpleDRAM.py (UNKNOWN) > > View Diff <http://reviews.gem5.org/r/1927/diff/> > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
