> On June 25, 2013, 10:12 a.m., 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 25, 2013, 10:12 a.m., Andreas Hansson wrote: > > src/mem/simple_dram.hh, line 435 > > <http://reviews.gem5.org/r/1927/diff/2/?file=36307#file36307line435> > > > > 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. > On June 25, 2013, 10:12 a.m., Andreas Hansson wrote: > > src/mem/SimpleDRAM.py, line 121 > > <http://reviews.gem5.org/r/1927/diff/2/?file=36306#file36306line121> > > > > 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 25, 2013, 10:12 a.m., Andreas Hansson wrote: > > src/mem/SimpleDRAM.py, line 124 > > <http://reviews.gem5.org/r/1927/diff/2/?file=36306#file36306line124> > > > > 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? - Amin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1927/#review4469 ----------------------------------------------------------- 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
