FPGA VGA Controller for 8-bit computer

[nockieboy]

Check out line 412:

Code: [Select]

    line_gen_starter     = !reset && ( !fifo_cmd_rdy_n && !geo_run ) && ( command_in[7:5] == 3'd0 && command_in[3:0] != 4'd0 )    ; // for now, only the draw line command

Now you just need to delay the ' line_gen_starter ' by 1 clock and everything should be honky dory.
Now tell me where you will be moving that line to to that?

I've moved it into the asynchronous always_ff block and changed it to a register assignment:

Code: [Select]

line_gen_starter    <= !reset && ( !fifo_cmd_rdy_n && !geo_run ) && ( command_in[7:5] == 3'd0 && command_in[3:0] != 4'd0 ) ; // for now, only the draw line command


[BrianHG]

Ok, does the sorting work?
Did you not forget to remove the '!reset' from the line.
Is it inside the !draw_busy ?
Did you add a clear in the if(reset)?

[nockieboy]

Ok, does the sorting work?

Hang on a mo - am I wiring up the poly_sort module correctly?  I'm not sure the x_in and y_in inputs are right (or will work as expected), but I really don't know how to get four sets of values back out of the module.... what do I connect to x_sort and y_sort?

Code: [Select]

poly_sort sorter (
// inputs
    .clk           ( clk          ),
    .enable        ( !draw_busy   ), // !pixel_writer busy input
    .bypass        ( 1'b0         ), // pass x_in/y_in values straight to x_sort/y_sort - no sorting
    .shape_quad    ( geo_shape[2] ), // sorts y_in[0-2] (3 points) if LOW, y_in[0-3] (4 points) if HIGH
    .x_in          ( '{ x[0], x[1], x[2], x[3] } ), // values to sort
    .y_in          ( '{ y[0], y[1], y[2], y[3] } ), // values to sort
//outputs
    .x_sort        ( ), // sorted value output
    .y_sort        ( ), // sorted value output
    .y-min         ( ),
    .y-max         ( ),
    .x-min         ( ),
    .x-max         ( ),
    .all_x_equal   ( ), // tells the linegens to skip a filled geometric shape and just draw 1 single line from y-min to y-max
    .all_y_equal   ( )  // tells the linegens to skip a filled geometric shape and just draw 1 single line from x-min to x-max
);


Did you not forget to remove the '!reset' from the line.
Is it inside the !draw_busy ?
Did you add a clear in the if(reset)?

Yep. 

[nockieboy]

Have just connected the poly_sort module in a way where the outputs copied the format of the inputs, and it compiles.

Problem is, Fmax on clk[2] has dropped to 113.22 MHz. 

[BrianHG]

Have just connected the poly_sort module in a way where the outputs copied the format of the inputs, and it compiles.

Problem is, Fmax on clk[2] has dropped to 113.22 MHz. 

I cant compile your code.  I get:

Code: [Select]

Error: Port "lineIndex" does not exist in macrofunction "linegen_3"
Error: Port "lineIndex" does not exist in macrofunction "linegen_2"
Error: Port "lineIndex" does not exist in macrofunction "linegen_1"
Also, why:

Code: [Select]

poly_sort sorter (
// inputs
    .clk           ( clk          ),
    .enable        ( !draw_busy   ), // !pixel_writer busy input
    .bypass        ( 1'b0         ), // pass x_in/y_in values straight to x_sort/y_sort - no sorting
    .shape_quad    ( geo_shape[2] ), // sorts y_in[0-2] (3 points) if LOW, y_in[0-3] (4 points) if HIGH
    .x_in          ( '{ x[0], x[1], x[2], x[3] } ), // values to sort
    .y_in          ( '{ y[0], y[1], y[2], y[3] } ), // values to sort
//outputs
    .sorted_x      ( '{ x_sort[0], x_sort[1], x_sort[2], x_sort[3] } ), // sorted value output
    .sorted_y      ( '{ y_sort[0], y_sort[1], y_sort[2], y_sort[3] } ), // sorted value output
    .y_min         ( ),
    .y_max         ( ),
    .x_min         ( ),
    .x_max         ( ),
    .all_x_equal   ( ), // tells the linegens to skip a filled geometric shape and just draw 1 single line from y-min to y-max
    .all_y_equal   ( )  // tells the linegens to skip a filled geometric shape and just draw 1 single line from x-min to x-max
);
Why not:

Code: [Select]

poly_sort sorter (
// inputs
    .clk           ( clk          ),
    .enable        ( !draw_busy   ), // !pixel_writer busy input
    .bypass        ( 1'b0         ), // pass x_in/y_in values straight to x_sort/y_sort - no sorting
    .shape_quad    ( geo_shape[2] ), // sorts y_in[0-2] (3 points) if LOW, y_in[0-3] (4 points) if HIGH
    .x_in          ( x ), // values to sort
    .y_in          ( y ), // values to sort
//outputs
    .sorted_x      ( x_sort ), // sorted value output
    .sorted_y      ( y_sort ), // sorted value output
    .y_min         ( ),
    .y_max         ( ),
    .x_min         ( ),
    .x_max         ( ),
    .all_x_equal   ( ), // tells the linegens to skip a filled geometric shape and just draw 1 single line from y-min to y-max
    .all_y_equal   ( )  // tells the linegens to skip a filled geometric shape and just draw 1 single line from x-min to x-max
);

You will most likely need to create a command path from:

the output fifo function and command parameters

Into 'poly_sort'

command parameters and sorted coordinates out

Into a either the linegens or a new module which takes in the commands and runs the linegens.

Out of that module and into the blitter

out of the blitter to the output of the geometry_xy_plotter's output port into the address generator.

Each module having a clock delay, or in the case of the last 2 modules multiple clock delays.

This is the only way we can get a grip on improving the FMAX and if necessary in the future, adding an additional clock latency.

[nockieboy]

Have just connected the poly_sort module in a way where the outputs copied the format of the inputs, and it compiles.

Problem is, Fmax on clk[2] has dropped to 113.22 MHz. 

I cant compile your code.  I get:

Code: [Select]

Error: Port "lineIndex" does not exist in macrofunction "linegen_3"
Error: Port "lineIndex" does not exist in macrofunction "linegen_2"
Error: Port "lineIndex" does not exist in macrofunction "linegen_1"

Sorry, you haven't got the latest line_generator.sv with the fix for the de-synched linegens.  Will take a look at rest of your post shortly. 

[nockieboy]

Here's my current test bench with the latest versions of line_generator and geo_xy_plotter.

I've got the cmd_data pipe set up from the FIFO, through poly_sort and into the linegens currently (best I can tell it simulates okay).  Quartus II is quoting an Fmax of 104.41 MHz.

If I'm reading anything correctly from the compilation report, it seems that the sort in poly_sort is causing the issue - does the IF structure need to be simplified or expanded further?

[BrianHG]

Ok, it's the quadrilateral sorting creating too much logic as when I remove it and just use the triangle sort, your FMAX's limiting factor is in the linegens instead.  It needs to be sorted in 2 steps.

Also, we need to fix up the command pipe as all it's functions like bypassing the sort and the geo-shape and color are not being used by the next function.

AND, the command pipe pass through (lines 643-644) also needs to be after line 646.....

I recommend keeping the triangle sort as the first stage in the pipe, then for the second stage, if a quad it selected, only if y[2] >= y[3], swap xy[2] & xy[3], or, if y[0] >= y[3], then swap xy[0] and xy[3].
Remember, the middle 2 coordinates don't need their y sorting if they 3 have already been sorted on the previous clock.

I'm assuming you are sorting y[0] minumum, y[3] maximum.

[nockieboy]

Also, we need to fix up the command pipe as all it's functions like bypassing the sort and the geo-shape and color are not being used by the next function.

How do you mean?

AND, the command pipe pass through (lines 643-644) also needs to be after line 646.....

Ah, okay, done.  I thought that it should be passing through cmd_data whether poly_sort is enabled or not though?

I recommend keeping the triangle sort as the first stage in the pipe, then for the second stage, if a quad it selected, only if y[2] >= y[3], swap xy[2] & xy[3], or, if y[0] >= y[3], then swap xy[0] and xy[3].
Remember, the middle 2 coordinates don't need their y sorting if they 3 have already been sorted on the previous clock.

Okay... so... pipelining the sort into a two-stage process... 

So instead of if (~shape_quad) begin starting off the triangle sorting, instead it'll be if ( !step ) begin, where step alternates between 0 and 1 each clock. Then the quad sort is done when step is 1?  Is there a better way?

Also, I'll need to increase the cmd_data pipeline by another step to delay it by 2 clocks if a quad is being sorted?

I'm going to need some sort of output to let the linegen's (or geo_xy_plotter) know that there is valid data?

I'm assuming you are sorting y[0] minumum, y[3] maximum.

Yep. 

[BrianHG]

You have the first triangle sort,

cmd_in -> cmd_mid_pipe &
in -> tri-sort

then continue

cmd_mid_pipe -> pipe_out &
tri-sort -> quad sort.

Dont forget the the first bypass if gets its decision from the cmd_in
and the trisort->quadsort bypass gets its bypass decision from the cmd_mid_pipe.

You are also need to input and operate based on a execute command ready and a reset input which only needs to clear the command stages.  Don't worry about all the xy[ # ] mid stages in the reset.

[nockieboy]

120.32 MHz Fmax.   (Assuming I've pipelined the sorting correctly.)

[BrianHG]

How is that a pipe?
you are going straight from in to sorted & in to sorted again?

what happened to going from in to sort stage #1, then sort stage #1 to sorted output?

Also, in the reset, reset the output command & output cmd_ready...

[nockieboy]

How is that a pipe?
you are going straight from in to sorted & in to sorted again?

what happened to going from in to sort stage #1, then sort stage #1 to sorted output?

Oh yeah.   Must have attached the module to the post before I'd finished it. 

Okay, Fmax is up over 128 MHz but the simulation output doesn't look too good.  I'm doing something wrong somewhere.

[BrianHG]

You have structural command problem throughout lines 664-680.

Like, why do you still have a bypass.
And why are you beginning a sort instead of a bypass if the command shape to be drawn is anything else other than a FILLED TRIANGLE, or a FILLED QUADRILATERAL.  And in the second case, only a filled QUADRILATERAL.

Also, if the y[0] to y[2] are all equal in filled triangle, or, y[0] through y[3] are all equal if quadrilateral, think about changing the command into a DRAW LINE, and make x[0] the lowest x[ # ] in the coordinates and make x[ # ] the highest x in the used coordinates.

Also, same if all X [ # ] are equal.  Change the command into a DRAW LINE and make y[0] the lowest used Y and make y[1] the highest Y.  This one may be simplified by making xy[0] the lowest Y and xy[1] the highest Y by default and changing the coordinate order in the line drawing engine.

Next, you also need to take care of 570-591 as these need to be part of a new line drawing module as they need to receive commands and what functions to draw after this sorting module.  This module will now initiate the linegens driving code and output a pixel ready plus XY coordinates + color + command.

[nockieboy]

You have structural command problem throughout lines 664-680.

Like, why do you still have a bypass.

And why are you beginning a sort instead of a bypass if the command shape to be drawn is anything else other than a FILLED TRIANGLE, or a FILLED QUADRILATERAL.  And in the second case, only a filled QUADRILATERAL.

Also, if the y[0] to y[2] are all equal in filled triangle, or, y[0] through y[3] are all equal if quadrilateral, think about changing the command into a DRAW LINE, and make x[0] the lowest x[ # ] in the coordinates and make x[ # ] the highest x in the used coordinates.

Also, same if all X [ # ] are equal.  Change the command into a DRAW LINE and make y[0] the lowest used Y and make y[1] the highest Y.  This one may be simplified by making xy[0] the lowest Y and xy[1] the highest Y by default and changing the coordinate order in the line drawing engine.

Okay, fair bit for me to be getting on with here.  So I need to get rid of the bypass input and replace its IF conditional for one that checks for line conditions?

Only filled triangles and filled quads to be sorted?

Next, you also need to take care of 570-591 as these need to be part of a new line drawing module as they need to receive commands and what functions to draw after this sorting module.  This module will now initiate the linegens driving code and output a pixel ready plus XY coordinates + color + command.

So this will be the module that sits between the poly_sort and blitter in the cmd pipeline?  The LUTs will need to be moved into this new module as well?

[BrianHG]

correct, the bypass was originally to be decoded outside the module from the command which was never sent through.  This is no longer the case and you need to evaluate 2 different commands at 2 different stages in the pipe.

Correct on number 2 as well.
#2 will also receive the command and pipe them through to generate it's output.
#2 also now generates the geometry 'running' command to pause everything else in the pipeline until the shape it is working on is complete.

[nockieboy]

Okay, if I have any time I'll have a go at this tomorrow afternoon.  This sorting feature seems to be a bit of a bottomless rabbit hole, though! 

[BrianHG]

Okay, if I have any time I'll have a go at this tomorrow afternoon.  This sorting feature seems to be a bit of a bottomless rabbit hole, though! 

It not that it is a bottomless hole.  You are adding logic & you need to space/step thing out in steps which can keep the FMAX at the top.  It is good practice for you as I did the same thing in the address generator for you awhile back.

Learning to compartmentalize and sequence your functions as such removes dead clock cycles when done right.
You are now creating a geometry graphics pipeline with stand alone modules, forwarding the command instruction as each draw command feeds through.  This will allow you to sequentially add in more functions into that command pipe as time goes on.

We did this with the 15 port ram where we fed through the bitplane/palette color mixing settings and sub-pixel selection with those auxiliary ports in parallel with the read address.

[BrianHG]

Maybe you misunderstand, just make the 2 stage sort with proper command pass thru first.
The added command interpretation and change to line is used to optimize flat filled triangles and flat filled quads.
I told you all this so you code the module expecting to add this optimization.

[nockieboy]

I know I've been silent for a day or so - have been busy with work and haven't been able to get anything done.  Here's where I'm at currently (latest geo_xy_plotter module attached).

Lines 662-671 have been updated to remove the old bypass value check and changed to only bypass the sorting if the shape is not a triangle or filled quad.

When I get a sec, I'll make a start on the next bit - optimising to line drawing if all points in an axis are identical.

What I meant with the 'rabbit hole' comment was that what started out as a simple sorting function seems to have turned into a multi-stage, command pipeline-editing optimiser  that probably should be in a module file all of its own. 

[BrianHG]

Ok, here we go:

Lines 662 to 669:
You cannot bypass/skip a clock cycle from in to out.

Line 672, you all of a sudden should not to decide to register the cmd_rdy_in if earlier you didn't for the pass-through which are still legitimate commands, just the coordinates arent sorted.

Line 740,  This isn't an else.  What happens if 2 sequential commands come in, once one goes through the first stage, then through the second but there is also another command ready at the first, this second stage will be ignored.

Ok, I'll re-write this in a way which will actually allow you to add additional processing steps at will without the confusing labels and middle sequences.

[BrianHG]

Redone!

Well, almost, I just need to add the raster-fill.
Please test these new units as all the draw commands without a fill should work.

I'll add the raster fill tomorrow.

Things done:
Reverted the line generator to a simple dumb one with nothing more than a 'pause' feature.
Made the input command sort, sort everything and now it also sets the sequence size for each linegen.
FMAX (geometry processor related) should no longer be a problem.
Made provisions for inserting the copy pixel blitter for completion on Tuesday.

***CHANGE: I made the max X&Y coordinate limiter = only pixel coordinates below the max setting are written to GPU ram, it is no longer the top pixel coordinate allowed.  This way, if max X&Y is set to 0, the geometry plotter cannot perform a pixel write protecting your GPU ram from any writes.

[nockieboy]

Wow! Thank you.    Seems to work just fine on the hardware  , buuut.... I had a look at the compilation report to see what the new Fmax would be for clk[2] and got this:



clk[2]'s Fmax is down to 104.6 MHz?  Is that right or have I got a compiler setting that needs changing again? (I haven't changed anything - just copy-pasted the new line gen and geo_xy_plotter modules into the existing GPU project.)

I've attached the Worst-Case Timing Paths report in case it's any use.  From my interpretation, it looks like Pixie's FIFO output to the poly_sort module isn't happy. 

[BrianHG]

How strange, this is what I get:



Take a look at the attached project.
(Backup your previous work or save it under a new name)

[nockieboy]

Must be a compiler setting then as the version of the project you uploaded compiles with exactly almost the same Fmax results as you're getting.  How odd - I was sure I hadn't changed anything but the results speak for themselves. 

EDIT:

I'm getting clk[0] Fmax = 125.94 MHz, clk[2] Fmax = 130.72 MHz.  Not quite the same as what you're getting.  Is that usual or is the Fmax sensitive to the host computer the project is compiled on?