----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1927/#review4469 -----------------------------------------------------------
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? src/mem/SimpleDRAM.py <http://reviews.gem5.org/r/1927/#comment4188> Do we need it per device? Why not combined? src/mem/SimpleDRAM.py <http://reviews.gem5.org/r/1927/#comment4189> 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? src/mem/simple_dram.hh <http://reviews.gem5.org/r/1927/#comment4190> I'd suggest to make all these const - Andreas Hansson On June 24, 2013, 6:01 a.m., Amin Farmahini wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1927/ > ----------------------------------------------------------- > > (Updated June 24, 2013, 6:01 a.m.) > > > Review request for Default. > > > 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. > > > Diffs > ----- > > src/mem/simple_dram.cc UNKNOWN > src/mem/simple_dram.hh UNKNOWN > src/mem/SimpleDRAM.py UNKNOWN > > Diff: http://reviews.gem5.org/r/1927/diff/ > > > Testing > ------- > > None > > > Thanks, > > Amin Farmahini > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
