Index: rtl/vid_ctl/vid_control.v =================================================================== --- rtl/vid_ctl/vid_control.v (revision 234) +++ rtl/vid_ctl/vid_control.v (working copy) @@ -46,8 +46,8 @@ Patrick McNamara -- wpm@openhardwarefoundation.net Copyright: - Copyright 2006-2007, Timothy Miller - howard.parkin@gmail.com - Copyright 2006-2007, Howard Parkin - theosib@gmail.com + Copyright 2006-2007, Timothy Miller - theosib@gmail.com + Copyright 2006-2007, Howard Parkin - howard.parkin@gmail.com Copyright 2006-2007, Patrick McNamara - wpm@openhardwarefoundation.net Comments: Index: rtl/oga1/s3/s3_top_level.v =================================================================== --- rtl/oga1/s3/s3_top_level.v (revision 234) +++ rtl/oga1/s3/s3_top_level.v (working copy) @@ -141,7 +141,9 @@ wire [31:0] eng_wdata; wire eng_do_write; - +// This is the interface signal set for the memory arbiter. +// It takes 27 bit addresses of 64 bit words (total of 1GB) +// And sends data out to the four memory controller. wire [26:0] br2arb_addr_in; wire [63:0] br2arb_data_in; wire [7:0] br2arb_bytes_in; @@ -298,7 +300,11 @@ .reg_data(eng_wdata), .reg_do_write(eng_do_write && ...), - .br2arb_addr(br2arb_addr_out >> 2), + .br2arb_addr(br2arb_addr_out >> 2), + // Hmm... this seems suspicious. + // We are accepting the address of a 64 bit word. + // br2arb_addr_out is 27 bits, so we do need to + // get rid of two, but shouldn't they be MSBs? .br2arb_wdata(br2arb_data_out), .br2arb_bytes(br2bar_bytes_out), .br2arb_do_read(br2arb_do_read_out), Index: rtl/oga1/s3/top_video_out.v =================================================================== --- rtl/oga1/s3/top_video_out.v (revision 234) +++ rtl/oga1/s3/top_video_out.v (working copy) @@ -106,6 +106,9 @@ // Double rate to 165 +// The 165 deals with actually sending data to the monitor +// correctly -- we just pack the pixels and send to the dual +// DVI inputs on the 165. reg [29:0] out0, out1; always @(posedge clock_2x) begin if (phase_2x) begin @@ -121,6 +124,8 @@ // Pipeline Pixels for Output +// This sends out the out1 signal (output RGB[23:0] pixels) at +// the correct phase in relation to the out0 signal, right? reg [29:0] out0_n, out1_p; reg [2:0] sig2, sig3; reg xmit_top_vsync, xmit_top_hsync,xmit_top_de; Index: rtl/oga1/s3/bot_video_out.v =================================================================== --- rtl/oga1/s3/bot_video_out.v (revision 234) +++ rtl/oga1/s3/bot_video_out.v (working copy) @@ -74,6 +74,9 @@ // Double rate to 165 +// The 165 deals with actually sending data to the monitor +// correctly -- we just pack the pixels and send to the dual +// DVI inputs on the 165. reg [23:0] out0, out1; always @(posedge clock_2x) begin if (phase_2x) begin @@ -90,6 +93,8 @@ // Pipeline Pixels for Output +// This sends out the out1 signal (output RGB[23:0] pixels) at +// the correct phase in relation to the out0 signal, right? reg [23:0] out0_n, out1_p; reg [2:0] sig2, sig3; reg xmit_bot_vsync, xmit_bot_hsync,xmit_bot_de; Index: rtl/oga1/s3/s3_bridge.v =================================================================== --- rtl/oga1/s3/s3_bridge.v (revision 234) +++ rtl/oga1/s3/s3_bridge.v (working copy) @@ -74,6 +74,8 @@ // Busy +// We can't receive data if there is nowhere to put it! +// Since there is arbitration, we need a FIFO, which fills. always @(posedge clock) begin bridge_busy <= mem_nearly_full; end @@ -107,6 +109,9 @@ b_rcount: rcount <= bridge_ad_in_d; b_write: begin mem_addr <= address[27:1]; // shift correct? + // No. We are reading in 64 bit words. + // Looking at s3_top_level, it should be + // address[28:2]. address is the raw address. // Cheat by not combining adjacent writes // This should be easy to optimize later mem_data <= {bridge_ad_in_d, bridge_ad_in_d}; @@ -122,11 +127,13 @@ if (rcount[6:1]) begin mem_addr <= address[27:1]; // shift? + // Same here, should be address[28:2]. mem_do_write <= 0; mem_do_read <= 1; mem_enq <= target[TARGET_MEM]; - address[6:1] <= address[6:1] + 1; + address[6:1] <= address[6:1] + 1; // For a similar reason to above, + // this should change as well. rcount[6:1] <= rcount[6:1] - 1; end end @@ -179,6 +186,8 @@ end // Dequeue when we're sending the top half +// This is because we are splitting the 64 bit +// data word into two 32 bit writes for the bridge. assign mem_rdata_deqA = return_slice == 1; assign mem_rdata_deqB = return_slice == 3; assign mem_rdata_deqC = return_slice == 5; Index: rtl/oga1/s3/arbiter.v =================================================================== --- rtl/oga1/s3/arbiter.v (revision 234) +++ rtl/oga1/s3/arbiter.v (working copy) @@ -39,6 +39,7 @@ // Read end of request queue from bridge // This is only a 32-bit interface + // No, this is 64 bits. Data to the memory controller is 64 bits wide. input [24:0] br_addr, input [63:0] br_wdata, input [7:0] br_bytes, @@ -180,6 +181,9 @@ wire refresh_access = refresh_pending; // Scheduler +// Highest priority is obviously refresh. +// Video outputs are second, because they are timing sensitive. +// Finally, PCI data is not really timing sensitive and waits. reg allow_pci, allow_vid0, allow_vid1, allow_refresh; always @(posedge clock or negedge reset_) begin if (!reset_) begin @@ -200,6 +204,8 @@ allow_vid1 <= 0; end else if (vid1_access) begin allow_vid0 <= 1; + // Why is vid0 allowed? It isn't accessing. + // Won't this mess up cmd_mem below? allow_pci <= 0; allow_refresh <= 0; allow_vid1 <= 1; @@ -228,6 +234,19 @@ wire vid0_cmd = allow_vid0 && vid0_valid; wire vid1_cmd = allow_vid1 && vid1_valid; +// This is the memory command signal. The different commands are: +// 1) mem_lmr -- load the memory load mode register +// 2) mem_precharge -- precharge all +// 3) mem_refresh -- refresh the bank (or row??) +// 4) mem_read -- read a word +// 5) mem_write -- write a word +// lmr, precharge, and refresh are run by the arbiter and are critical. +// read is used by the PCI (bridge) interface and mostly by the video signals. +// write is used by the PCI interface. + +// What happened to precharging a row -- auto precharge? I assume we are using +// auto precharge, so doesn't column[9] need to be set? + assign cmd_mem = ({3{lmr_cmd}} & mem_lmr) | ({3{precharge_cmd}} & mem_precharge) | @@ -237,13 +256,18 @@ ({3{vid0_cmd}} & mem_read) | ({3{vid1_cmd}} & mem_read); +// bank_mem is the bank currently being accessed. +// It is the upper two bits of the address of the 64 bit words. + assign bank_mem = - ({2{lmr_cmd}} & lmr_data[14:13]) | + ({2{lmr_cmd}} & lmr_data[14:13]) | // configure the mode of the bank ({2{pci_read_cmd}} & br_addr[22:21]) | ({2{pci_write_cmd}} & br_addr[22:21]) | ({2{vid0_cmd}} & vid0_addr[22:21]) | ({2{vid1_cmd}} & vid1_addr[22:21]); - + +// row_mem is the row currently being read/written + // 13-bit row assign row_mem = ({13{pci_read_cmd}} & br_addr[20:8]) | @@ -265,6 +289,9 @@ assign wbytes_mem = br_bytes; assign rtag_mem = {pci_read_cmd, vid0_cmd, vid1_cmd}; + // This tells the memory_wrapper, and therefore memctl_top, what is being sent. + // This is sent through from write to read and lets the arbiter know who requested + // data once it finally comes. // Return path for read data