FPGA VGA Controller for 8-bit computer

[BrianHG]

Found a bug in the 'bitplane_to_raster' module.

Line 146: if (x_out[3])...
I'll let you figure out the bug...

Ooh.. shouldn't that be x_out[2]?

Yes.
But, don't forget this: https://www.eevblog.com/forum/fpga/fpga-vga-controller-for-8-bit-computer/msg2821728/#msg2821728

[nockieboy]

Your pixels are still off by 1 or 2 according to the image I see.

The original code has 0 delay on the bit selection.  This new code has 1, so long as you remove all the internal delays.  You need the other modules to take that into effect.

Right, fixed that by changing the x_in input to the bitplane_to_raster instance from dly6_disp_x to dly5_disp_x. 

Remember my comment after this fix post of yours:
https://www.eevblog.com/forum/fpga/fpga-vga-controller-for-8-bit-computer/msg2816848/#msg2816848

Doing it the wrong way will come back to haunt you later....  Like soon...

Sorry, you've lost me there.  Is the fix you've linked to above incorrect then?  Or is the fault with x_out[3] linked to this previous fix? Should it be x_in[3] instead to remove the one clock delay?

[BrianHG]

     You have 2 problems with the code.  Changing the bit from 3 to 2 fixes 1 problem.  Playing with the pixel bitplane mode setting with the on screen text should have shown you a difference and you would have noticed the problem with the [3].
     Now, your home made delay 6 to 5 patch trick will not work with the new address generators as there will no longer be any x position delays to tap from once we go to the new address generators.

[BrianHG]

Ok, the ' .rden_b() ' doesn't do what I want, we must use the ' .aclr1 () ' for all the host memory ports.

Do this exactly:

Loose your rden_b port, and make a sclr_n_rd_b input and inside the 'ram_INTEL' module, add this...

reg aclr_delay,aclr_delay2;

on the altsyncram add this port:
            .aclr0 ( 1'b0 ),
            .aclr1 (aclr_delay2)

make sure you don't have doubles and the rden_b is now forced back to ' (1'b1) '

change these defparams:

      altsyncram_component.outdata_aclr_a = "CLEAR0",
      altsyncram_component.outdata_aclr_b = "CLEAR1",

add this at the end of the memory before ' endmodule'

always @(posedge clk_b) begin
aclr_delay  <= ~sclr_n_rd_b;
aclr_delay2 <= aclr_delay;
end

[nockieboy]

Ok, the ' .rden_b() ' doesn't do what I want, we must use the ' .aclr1 () ' for all the host memory ports.

Do this exactly:

Loose your rden_b port, and make a sclr_n_rd_b input and inside the 'ram_INTEL' module, add this...

reg aclr_delay,aclr_delay2;

on the altsyncram add this port:
            .aclr0 ( 1'b0 ),
            .aclr1 (aclr_delay2)

make sure you don't have doubles and the rden_b is now forced back to ' (1'b1) '

change these defparams:

      altsyncram_component.outdata_aclr_a = "CLEAR0",
      altsyncram_component.outdata_aclr_b = "CLEAR1",

add this at the end of the memory before ' endmodule'

always @(posedge clk_b) begin
aclr_delay  <= ~sclr_n_rd_b;
aclr_delay2 <= aclr_delay;
end

Okay, done all that - I assume when you said, "we must use the ' .aclr1 () ' for all the host memory ports", you meant the palette memory ports and not the GPU memory as well??

Updated project files attached.

[nockieboy]

     You have 2 problems with the code.  Changing the bit from 3 to 2 fixes 1 problem.  Playing with the pixel bitplane mode setting with the on screen text should have shown you a difference and you would have noticed the problem with the [3].
     Now, your home made delay 6 to 5 patch trick will not work with the new address generators as there will no longer be any x position delays to tap from once we go to the new address generators.

So either I'm screwed or there's some solution you're alluding to that is just beyond my grasp with the meagre time I have to focus on this currently? 

[BrianHG]

     Now, your home made delay 6 to 5 patch trick will not work with the new address generators as there will no longer be any x position delays to tap from once we go to the new address generators.

So either I'm screwed or there's some solution you're alluding to that is just beyond my grasp with the meagre time I have to focus on this currently? 

This isn't difficult.  Think carefully.  If you are shortening the delay of the 'x' counter position by "1" before you feed the input of the bitplane to raster module to fix a problem, then, is it possible that you have a delay in the currently used 'x' coordinates inside the bitplane to raster module which shouldn't be there?

[nockieboy]

This isn't difficult.  Think carefully.  If you are shortening the delay of the 'x' counter position by "1" before you feed the input of the bitplane to raster module to fix a problem, then, is it possible that you have a delay in the currently used 'x' coordinates inside the bitplane to raster module which shouldn't be there?

The only 'internal' delay to the x coordinates I can spot is the one I mentioned earlier? It's possible you read that message before I edited the comment in red into the post... 

Sorry, you've lost me there.  Is the fix you've linked to above incorrect then?  Or is the fault with x_out[3] linked to this previous fix? Should it be x_in[3] instead to remove the one clock delay?

[BrianHG]

Ok, the ' .rden_b() ' doesn't do what I want, we must use the ' .aclr1 () ' for all the host memory ports.

Do this exactly:

Loose your rden_b port, and make a sclr_n_rd_b input and inside the 'ram_INTEL' module, add this...

reg aclr_delay,aclr_delay2;

on the altsyncram add this port:
            .aclr0 ( 1'b0 ),
            .aclr1 (aclr_delay2)

make sure you don't have doubles and the rden_b is now forced back to ' (1'b1) '

change these defparams:

      altsyncram_component.outdata_aclr_a = "CLEAR0",
      altsyncram_component.outdata_aclr_b = "CLEAR1",

add this at the end of the memory before ' endmodule'

always @(posedge clk_b) begin
aclr_delay  <= ~sclr_n_rd_b;
aclr_delay2 <= aclr_delay;
end

Okay, done all that - I assume when you said, "we must use the ' .aclr1 () ' for all the host memory ports", you meant the palette memory ports and not the GPU memory as well??

Updated project files attached.

  The .aclr1() trick wont work as well, make this change instead...

Change the output to a reg:
...
   output reg [7:0] data_out_b
...
Make a new wire
...
wire [7:0] pre_data_out_b;
...
change the altsyncram IOs:
...
            .q_b (pre_data_out_b),
            .aclr0 (1'b0),
            .aclr1 (1'b0),
...
change the altsyncram defparams:
...
      altsyncram_component.outdata_aclr_a = "NONE",
      altsyncram_component.outdata_aclr_b = "NONE",
      altsyncram_component.outdata_reg_a = "CLOCK0",
      altsyncram_component.outdata_reg_b = "UNREGISTERED",
...
create this true synchronous read with clear data before the 'endmodule':
...
always @(posedge clk_b) begin
aclr_delay  <= rd_en_b;
     if (aclr_delay) data_out_b  <= pre_data_out_b;
     else               data_out_b  <= 0;
end
...

This will keep a 2 clock read, with good performance, and create a synchronous '0' with the read 2 clock timing when the read enable is low.

[BrianHG]

This isn't difficult.  Think carefully.  If you are shortening the delay of the 'x' counter position by "1" before you feed the input of the bitplane to raster module to fix a problem, then, is it possible that you have a delay in the currently used 'x' coordinates inside the bitplane to raster module which shouldn't be there?

The only 'internal' delay to the x coordinates I can spot is the one I mentioned earlier? It's possible you read that message before I edited the comment in red into the post... 

Look carefully at the:
...
line 86               if (ram_byte_in[(~x_out[2:0])] == 1'b1) begin
...
line 112               case (x_out[2:1])
...
line 146               if (x_out[2])
...
line 180               if (ram_byte_in[(~x_out[2:0])] == 1'b1) begin
...

Is it possible that you have a delay in the currently USED 'x' coordinates inside the bitplane to raster module which shouldn't be there?

Prepare to bang your head and see why I was banging mine with the comment:

https://www.eevblog.com/forum/fpga/fpga-vga-controller-for-8-bit-computer/msg2816848/#msg2816848
   Arrrrrrggg, how did you do that?  You found a different, yet perfect solution to a weird problem I never thought would end up like this....

Ok...  Fine...  Great...

Next...

If I need to make 1 more comment on this, your gonna feel worse...

[BrianHG]

Look at line #37:
      x_out               <= x_in;

[nockieboy]

The .aclr1() trick wont work as well, make this change instead...

Change the output to a reg:
...
   output reg [7:0] data_out_b
...
Make a new wire
...
wire [7:0] pre_data_out_b;
...
change the altsyncram IOs:
...
            .q_b (pre_data_out_b),
            .aclr0 (1'b0),
            .aclr1 (1'b0),
...
change the altsyncram defparams:
...
      altsyncram_component.outdata_aclr_a = "NONE",
      altsyncram_component.outdata_aclr_b = "NONE",
      altsyncram_component.outdata_reg_a = "CLOCK0",
      altsyncram_component.outdata_reg_b = "UNREGISTERED",
...
create this true synchronous read with clear data before the 'endmodule':
...
always @(posedge clk_b) begin
aclr_delay  <= rd_en_b;
     if (aclr_delay) data_out_b  <= pre_data_out_b;
     else               data_out_b  <= 0;
end
...

This will keep a 2 clock read, with good performance, and create a synchronous '0' with the read 2 clock timing when the read enable is low.

All done.  Updated dual_port_palette_RAM_INTEL.v below:

Code: [Select]

module dual_port_palette_ram_INTEL (

// inputs
input clock_a,
input clock_b,
input [7:0]  pixel_addr_in, // address_a
input [8:0]  host_address,
input [15:0] data_a,
input [7:0]  host_data_in,
input enable_a,
input host_enable,
input wren_a,
input host_wren,
input rden_a,
input rden_b,
input sclr_n_rd_b,

// outputs
output [15:0] pixel_out, // data_out_a
output reg [7:0]  host_data_out

);

parameter INIT_PALETTE = "palette_565.mif";

wire [7:0] pre_data_out_b;

reg aclr_delay;

// ****************************************************************************************************************************
// Dual-port palette RAM
// ****************************************************************************************************************************
altsyncram altsyncram_component (
.clocken0 (enable_a),
.clocken1 (host_enable),
.wren_a (wren_a),
.clock0 (clock_a),
.wren_b (host_wren),
.clock1 (clock_b),
.address_a (pixel_addr_in),
.address_b (host_address),
.data_a (data_a),
.data_b (host_data_in),
.q_a (pixel_out),
.q_b (pre_data_out_b),
.aclr0 (1'b0),
.aclr1 (1'b0),
.addressstall_a (1'b0),
.addressstall_b (1'b0),
.byteena_a (1'b1),
.byteena_b (1'b1),
.clocken2 (1'b1),
.clocken3 (1'b1),
.eccstatus (1'b0),
.rden_a (rden_a),
.rden_b (rden_b)
);

defparam
altsyncram_component.address_reg_b = "CLOCK1",
altsyncram_component.clock_enable_input_a = "NORMAL",
altsyncram_component.clock_enable_input_b = "NORMAL",
altsyncram_component.clock_enable_output_a = "NORMAL",
altsyncram_component.clock_enable_output_b = "NORMAL",
altsyncram_component.indata_reg_b = "CLOCK1",
altsyncram_component.init_file = INIT_PALETTE,
altsyncram_component.init_file_layout = "PORT_A",
altsyncram_component.intended_device_family = "Cyclone IV",
altsyncram_component.lpm_type = "altsyncram",
altsyncram_component.numwords_a = 256,
altsyncram_component.numwords_b = 512,
altsyncram_component.operation_mode = "BIDIR_DUAL_PORT",
altsyncram_component.outdata_aclr_a = "NONE",
altsyncram_component.outdata_aclr_b = "NONE",
altsyncram_component.outdata_reg_a = "CLOCK0",
altsyncram_component.outdata_reg_b = "UNREGISTERED",
altsyncram_component.power_up_uninitialized = "FALSE",
altsyncram_component.read_during_write_mode_port_a = "OLD_DATA",
altsyncram_component.read_during_write_mode_port_b = "OLD_DATA",
altsyncram_component.widthad_a = 8,
altsyncram_component.widthad_b = 9,
altsyncram_component.width_a = 16,
altsyncram_component.width_b = 8,
altsyncram_component.width_byteena_a = 1,
altsyncram_component.width_byteena_b = 1,
altsyncram_component.wrcontrol_wraddress_reg_b = "CLOCK1";

// ****************************************************************************************************************************

always @(posedge clock_b) begin

aclr_delay <= rd_en_b;
if (aclr_delay)
data_out_b <= pre_data_out_b;
else
data_out_b <= 1'b0;

end

endmodule


[nockieboy]

If I need to make 1 more comment on this, your gonna feel worse...

Look at line #37:
      x_out               <= x_in;

Darnit, you didn't give me time to reply before you posted another message! 

I'm using x_out in the code, which is delayed by a clock due to the line you've quoted above, when I should be using x_in.  I spotted that earlier and updated the reply I'd made, suggesting it as a modification, but you missed it?  Either that, or you're saying that I shouldn't be assigning x_in to x_out at all and it should just pass straight through without being registered?

[BrianHG]

Sorry, you've lost me there.  Is the fix you've linked to above incorrect then?  Or is the fault with x_out[3] linked to this previous fix? Should it be x_in[3] instead to remove the one clock delay?

Oooops, I saw the x_in[3] and focused on the '3' not the '_in'.  Yes, inside the 'if' your should only be using the x_in[] since it's the input which also needs comparing with the input data.  Everything should run in parallel...

Yes, you did get it earlier...

Ok, gotta do the 2-3 fixes to the main GPU_RAM module, then the address generators.

Next, do this: https://www.eevblog.com/forum/fpga/fpga-vga-controller-for-8-bit-computer/msg2818788/#msg2818788

Then we can tackle the address generator.

[nockieboy]

Ok, gotta do the 2-3 fixes to the main GPU_RAM module, then the address generators.

Sorry, just remind me what these 2-3 fixes were, exactly?  I'm working on making the gpu_dual_port_ram_INTEL module output 16-bits as per your linked previous post - are there other changes to be made as well?

[nockieboy]

@nockieboy, I hate to do this to you, but once you get the initial palette working, you will need to make a change to the 'multiport_gpu_ram.v' and 'gpu_dual_port_ram_INTEL.v'.  Now, I want to get this change right in 1 shot, so, we will need to go over it after the text palette is fully 100% functional without a single pixel in error...

Okay, I've made all the changes you've specified, but I'm getting the following error:

Error (272006): In altsyncram megafunction, when OPERATION_MODE parameter is set to BIDIR_DUAL_PORT, total number of bits of port A and port B must be the same

The RAM isn't happy with 8-bit and 16-bit ports.  I'd made a start making the host port 16-bit, planning to use the same technique to swap bytes around depending on the address's LSB, but then I realised that writing to the 16-bit port by an 8-bit host will be... messy... at best, and will corrupt the RAM contents at worst.  So I'm holding here to see if you have any pearls of wisdom?

Current project attached so you can review the changes made to the RAM so far.

[hamster_nz]

@nockieboy, I hate to do this to you, but once you get the initial palette working, you will need to make a change to the 'multiport_gpu_ram.v' and 'gpu_dual_port_ram_INTEL.v'.  Now, I want to get this change right in 1 shot, so, we will need to go over it after the text palette is fully 100% functional without a single pixel in error...

Okay, I've made all the changes you've specified, but I'm getting the following error:

Error (272006): In altsyncram megafunction, when OPERATION_MODE parameter is set to BIDIR_DUAL_PORT, total number of bits of port A and port B must be the same

The RAM isn't happy with 8-bit and 16-bit ports.  I'd made a start making the host port 16-bit, planning to use the same technique to swap bytes around depending on the address's LSB, but then I realised that writing to the 16-bit port by an 8-bit host will be... messy... at best, and will corrupt the RAM contents at worst.  So I'm holding here to see if you have any pearls of wisdom?

Current project attached so you can review the changes made to the RAM so far.

Did you change the width of the address line too? The address line for the 8-bit port needs to be one bit wider than the 16-bit port (as 16*1024 = 8*2048 = 16384 bits)

[BrianHG]

There were the primary mistakes:

Code: [Select]

altsyncram_component.numwords_a = NUM_WORDS /2,

altsyncram_component.widthad_a = ADDR_SIZE - 1 ,  // *************************** 1 less address since we are now 16 bit

altsyncram_component.init_file_layout = "PORT_B",      // *************** assign .mif file to layout 'B'


Too many other changes all at once.  Before making these changes, did everything else work properly?

Like, why did 'PIPE_DELAY' go from the proper value of 7 down to 6 from the last version to this version?
This would have messed up the display by 1 pixel.
I cannot track compound errors...

Too many other changes all at once...

Now, I've gone back 1 version, and just changes the ram_INTEL to make port 'A' 16bit, and use the assign 'mux' ram_data_a from 16bit to 8 bit out, and with 1 change of the addr[0], to the correct delay of addr[0], it works properly.  You will need to reverse the additional compound errors yourself as I cannot figure out why your latest  upload has numerous crap text errors and green background.

[nockieboy]

There were the primary mistakes:

Code: [Select]

altsyncram_component.numwords_a = NUM_WORDS /2,

altsyncram_component.widthad_a = ADDR_SIZE - 1 ,  // *************************** 1 less address since we are now 16 bit

altsyncram_component.init_file_layout = "PORT_B",      // *************** assign .mif file to layout 'B'



Ah, that's got it compiling again...

Too many other changes all at once.  Before making these changes, did everything else work properly?

Like, why did 'PIPE_DELAY' go from the proper value of 7 down to 6 from the last version to this version?

Yes, it was working fine - the PIPE_DELAY error was because I'd fixed the x_in/x_out issue and changed PIPE_DELAY back to 7, but because of a work distraction here, I came back and got confused about the original value and changed it back to 6. 

Now, I've gone back 1 version, and just changes the ram_INTEL to make port 'A' 16bit, and use the assign 'mux' ram_data_a from 16bit to 8 bit out..

I've made the changes you specified in the main post previously, including making the command bus 32 bit etc., have had to expand the pipelines from 8- to 16-bit according to the changes made to the data bus etc.  As far as I can tell, the changes are according to what you've specified.  I'd marked all the requested changes in the code with comments to make them easier to spot.

This is my output currently:

...and with 1 change of the addr[0], to the correct delay of addr[0], it works properly.  You will need to reverse the additional compound errors yourself as I cannot figure out why your latest  upload has numerous crap text errors and green background.

Well, it should be a blue background.  It compiles with no errors now, but the text is clearly corrupted.

[nockieboy]

Ignore the green background, I'd been messing with the default values. 

[BrianHG]

There were the primary mistakes:

Code: [Select]

altsyncram_component.numwords_a = NUM_WORDS /2,

altsyncram_component.widthad_a = ADDR_SIZE - 1 ,  // *************************** 1 less address since we are now 16 bit

altsyncram_component.init_file_layout = "PORT_B",      // *************** assign .mif file to layout 'B'



Ah, that's got it compiling again...

Too many other changes all at once.  Before making these changes, did everything else work properly?

Like, why did 'PIPE_DELAY' go from the proper value of 7 down to 6 from the last version to this version?

Yes, it was working fine - the PIPE_DELAY error was because I'd fixed the x_in/x_out issue and changed PIPE_DELAY back to 7, but because of a work distraction here, I came back and got confused about the original value and changed it back to 6. 

Noooooooooo.  The x_in/x_out fixes the:

Code: [Select]

.bg_colour( GPU_HW_Control_regs[10] ),
.x_in( dly6_disp_x ),
.colour_mode_in( GPU_HW_Control_regs[12][2:0] ),
.x_in( dly5_disp_x ),
 Back to the proper:
.x_in( dly6_disp_x ),

This bug had nothing to do with the 'PIPE_DELAY', which had the proper value of '7'.

The PIPE_DELAY went from 6 to 7 because the 'bitplane_to_raster' takes 1 clock cycle to create an output pixel where the older pixel character[~xsel] selection from the ram took 0 clock cycles.

Now, your garbage text, did you fix the 'addr[0]' so that the addr which selects the correct memory output byte is the same addr with the same byte we expect to receive from the altsyncram's memory output when the original address was fed in.

Remember, we are feeding 125 million new read addresses second.  Non stop.  The ram coming out keeps on changing 125 million times a second, even though it is 2 clocks behind.  Take this info into consideration when selecting the high and low byte.

(You still have other errors as even the green seems impossible.  I only re-wrote your code from the last version to what you were supposed to create, and it correctly worked with the 16 bit memory and 16 bit mux and 32 bit aux ports and I got perfect blue text.)

After the address generator, we will be modding the gpu_INTEL ram into a new wrapper module, running the the INTEL ram at 250MHz, making it's 2 ports into 4 ports at 125MHz.  And, we will be multiplying the 'multiport_gpu_ram' module 3 fold making a 15 read address ports for 15 parallel pixel reads, plus the 1 host_ port.  I hope you are up to the challenge.  (We would need 1GHz 8 bit static ram to replicate this, not counting the palette memory which would double that as we might do the same 15 ports there for true multiple degrees of translucency between all layers instead of between 2 layers, then just transparent color 0 / vs non-transparent.  Also in the lattice part, you will ran the ram at 500Mhz, in other words, you would need a 4GHz 8 bit static ram to replicate that display engine with it's 40 parallel read ports and 2x equivalent host_ ports.)

[nockieboy]

I've rolled back the 16-bit changes I made (except the 32-bit changes to the command bus - this isn't used yet so won't affect the output at the moment) and I'm getting a good display again.  Clearly something I'm doing with the 16-bit changes is incorrect.  Here's a summary of the changes I made: (NOTE: these are JUST the changes to the data bus, not the full code)

In multiport_gpu_ram.v:

Code: [Select]

// data buses (output)
output reg [15:0] data_out_0, // ****** changed to 16 bit width
output reg [15:0] data_out_1, // ****** changed to 16 bit width
output reg [15:0] data_out_2, // ****** changed to 16 bit width
output reg [15:0] data_out_3, // ****** changed to 16 bit width
output reg [15:0] data_out_4, // ****** changed to 16 bit width

wire [15:0] data_mux_out; // ****** changed to 16 bit width

reg [DEMUX_PIPE_TOP*16+15:0] data_pipe; // ****** changed to 16 bit width

data_pipe[15:0]                     <= data_mux_out[15:0]; // fill the first 16-bit word in the register pipe with data from RAM
data_pipe[DEMUX_PIPE_TOP*16+15:1*16] <= data_pipe[ (DEMUX_PIPE_TOP-1) *16+15:0*16]; // shift over the next 9 words in this 10 word, 16-bit wide pipe

data_out_0 <= data_pipe[MUX_0_POS*16+15:MUX_0_POS*16]; // ****** changed to 16 bit width
data_out_1 <= data_pipe[MUX_1_POS*16+15:MUX_1_POS*16];
data_out_2 <= data_pipe[MUX_2_POS*16+15:MUX_2_POS*16];
data_out_3 <= data_pipe[MUX_3_POS*16+15:MUX_3_POS*16];
data_out_4 <= data_pipe[MUX_4_POS*16+15:MUX_4_POS*16];

In gpu_dual_port_ram_INTEL:

Code: [Select]

output wire [15:0] data_out_a, // ****** changed to 16 bit data width

// ********************************************************************************
// * NEW SECTION to reverse byte order of read-only port if addressed LSB is 0
// ********************************************************************************

wire [15:0] ram_out_a;
assign data_out_a[15:8] = (addr_a[0] == 1'b0) ? ram_out_a[15:8] : ram_out_a[7:0];
assign data_out_a[7:0] = (addr_a[0] == 1'b0) ? ram_out_a[7:0]  : ram_out_a[15:8];

altsyncram altsyncram_component (
.address_a (addr_a[ADDR_SIZE - 1:1]), // ****** changed LSB from 0 to 1
.data_a (16'b0000000000000000), // ****** changed to 16 bit data width
.q_a (ram_out_a), // ****** changed from data_out_a to ram_out_a

altsyncram_component.numwords_a = NUM_WORDS / 2,   // ****** changed to NUM_WORDS / 2
altsyncram_component.widthad_a = ADDR_SIZE - 1,  // ****** changed to ADDR_SIZE - 1
altsyncram_component.width_a = 16, // ****** changed from 8 to 16
altsyncram_component.init_file_layout = "PORT_B",

In bitplane_to_raster.v:

Code: [Select]

input wire [7:0] ram_byte_in, // ****** changed to 16 bit width

These are all the changes I made (other than the cmd bus ones) - I've reverted all these changes and the output is fine again.  Any chance you could share the changes you've made to get it to work?

Noooooooooo.  The x_in/x_out fixes the:

Code: [Select]

.bg_colour( GPU_HW_Control_regs[10] ),
.x_in( dly6_disp_x ),
.colour_mode_in( GPU_HW_Control_regs[12][2:0] ),
.x_in( dly5_disp_x ),
 Back to the proper:
.x_in( dly6_disp_x ),

This bug had nothing to do with the 'PIPE_DELAY', which had the proper value of '7'.

Yes, realised this later.   

Now, your garbage text, did you fix the 'addr[0]' so that the addr which selects the correct memory output byte is the same addr with the same byte we expect to receive from the altsyncram's memory output when the original address was fed in.

Fix as in register it? Err... no?  Would this code not work then, unless addr_a[0] is registered?

Code: [Select]

assign data_out_a[15:8] = (addr_a[0] == 1'b0) ? ram_out_a[15:8] : ram_out_a[7:0];
assign data_out_a[7:0] = (addr_a[0] == 1'b0) ? ram_out_a[7:0]  : ram_out_a[15:8];


(You still have other errors as even the green seems impossible.  I only re-wrote your code from the last version, and it correctly worked with the 16 bit memory and 16 bit mux and 32 bit aux ports and I got perfect blue text.)

The green isn't an error, it's where I'd changed the default HW_regs settings and forgot to change them back before posting the project. 

[nockieboy]

Okay, at first glance I seem to have fixed the corrupt text.  Instead of using addr_a coming IN to the gpu_dual_port_ram_INTEL module, I switched it to the address going OUT (addr_out_a) - it's a clock behind, but it's a register so should stay valid long enough to lift the data from both bytes in the word... and it does. 

Obligatory pic:



EDIT: Latest project files attached.

[nockieboy]

After the address generator, we will be modding the gpu_INTEL ram into a new wrapper module, running the the INTEL ram at 250MHz, making it's 2 ports into 4 ports at 125MHz.  And, we will be multiplying the 'multiport_gpu_ram' module 3 fold making a 15 read address ports for 15 parallel pixel reads, plus the 1 host_ port.  I hope you are up to the challenge.  (We would need 1GHz 8 bit static ram to replicate this, not counting the palette memory which would double that as we might do the same 15 ports there for true multiple degrees of translucency between all layers instead of between 2 layers, then just transparent color 0 / vs non-transparent.  Also in the lattice part, you will ran the ram at 500Mhz, in other words, you would need a 4GHz 8 bit static ram to replicate that display engine with it's 40 parallel read ports and 2x equivalent host_ ports.)

   I hope I'm up to the challenge, as well!  I might have to fold and ask for more breadcrumbs to get me through this more quickly, though.  Can't believe I've wasted a day today hunting for (and thankfully fixing) two silly bugs. 

And, we will be multiplying the 'multiport_gpu_ram' module 3 fold making a 15 read address ports for 15 parallel pixel reads, plus the 1 host_ port.

I'm going to need a bigger FPGA!   It'll maybe stretch to doubling the multiport_gpu_ram module with it's ~33 KB of RAM, but certainly won't manage three or more such modules. 

[BrianHG]

Okay, at first glance I seem to have fixed the corrupt text.  Instead of using addr_a coming IN to the gpu_dual_port_ram_INTEL module, I switched it to the address going OUT (addr_out_a) - it's a clock behind, but it's a register so should stay valid long enough to lift the data from both bytes in the word... and it does. 

Obligatory pic:

(Attachment Link)

EDIT: Latest project files attached.

How did you come to the conclusion that the 'addr_out_a' is a clock behind the ram 'data_out_a'?
Does this image not represent the GPU_INTEL memory:

Count the number of clocks from address in to data out by counting the number of D-flipflops in the attached image...
How many do you count?
How many D-flipflops has the 'addr_out_a' gone through since the 'addr_a' input?

It's important you understand what's going on here.  All these pipes I made, though they all might not be used, the address generator will feed some values through them, IE coordinates, new memory to point to, expecting a ram read/or/pixel and these values/settings to be parallel at the output for everything to work properly at the next stage.  Without this parallel infrastructure, you will have code like in the current OSD generator's old junk like this obsolete patch work:

Code: [Select]


// **********************************************************************************************
// *** These delay pipes registers are explained in the 'assign's above
// **********************************************************************************************
dly1_disp_x <= disp_x;
dly2_disp_x <= dly1_disp_x;
dly3_disp_x <= dly2_disp_x;
dly4_disp_x <= dly3_disp_x;
dly5_disp_x <= dly4_disp_x;
dly6_disp_x <= dly5_disp_x;
dly7_disp_x <= dly6_disp_x;
dly8_disp_x <= dly7_disp_x;

dly1_disp_y <= disp_y;
dly2_disp_y <= dly1_disp_y;
dly3_disp_y <= dly2_disp_y;
dly4_disp_y <= dly3_disp_y;

dly1_letter <= letter;
dly2_letter <= dly1_letter;
dly3_letter <= dly2_letter;
dly4_letter <= dly3_letter;

dly1_dena   <= dena;
dly2_dena   <= dly1_dena;
dly3_dena   <= dly2_dena;
dly4_dena   <= dly3_dena;
dly5_dena   <= dly4_dena;
dly6_dena   <= dly5_dena;

// **********************************************************************************************
osd_ena_out <= dly4_dena; // This is used to drive a graphics A/B switch which tells when the OSD graphics should be shown
// It needs to be delayed by the number of pixel clocks required for the above memories
pixel_ena <= dly4_dena;
Which is about to be all deleted.  But, if you have a mistake somewhere in the 'multiport_gpu_ram' and it's parallel pipe, when we feed it a continuous address with embedded in the auxiliary pipe x coordinates and start and stop pixel coordinates to be displayed in parallel with 3 to 12 other address generator on every addr_in_# channel, you will never be able to back track and find any error at all if 1 single thing is out with this code.