FPGA VGA Controller for 8-bit computer

[BrianHG]

Ok, close...
the 'AUX' is the command.  Being 4 bits, we can feed 0 through 16, + 'draw_cmd_rdy' which says something valid has come to eat.

Code: [Select]

// |-AUX-|                                 |--COMMAND IN-| |COMMAND_DATA8|
// |-AUX-|                                 |-CMD-| |----COMMAND_DATA12---|

wire [7:0]  command_in     = draw_cmd[15:8];
wire [11:0] command_data12 = draw_cmd[11:0];
wire [7:0]  command_data8  = draw_cmd[7:0];
These do not exist here.

You code should look like this:

Code: [Select]

    if ( draw_cmd_rdy) begin
       case (aux_cmd_in)
               4'd01 : 
               4'd02 : 
               4'd03 : 
       endcase
   end

As for the memory address outputs...

reg complete_address_[w/r][24bits] = (DESTINATION/SOURCE BASE MEMORY ADDRESS +  ( y * screen_width + x ))

output wire memory_address_out = complete_address_[w/r][20bits] >> ( the correct amount of 16 bit words per pixel).
And don't forget to mute out the LSB address to 0 as it fools the 16 portGPU ram into addressing a 16bit word in 8 bit mode.

Remember, we are addressing 16 bits here, not 8 like in the MAGGIE.

we want other outputs.
memory_w/r_bit_size[3:0]   = number of bits to write/read  0..15 = 1..16bits
memory_w/r_bit_offset[3:0] = bit position, 0..15.

There's also a color output.

Your output should reflect what the pixel writer needs to operate.  Think of the things we need to say to the pixel writer and make an appropriate bus.  (Yes you can use your current outputs, but you need to tell us how you want to pack all the above required information into that bus like what I did on the output of the geometry xy plotter unit.)

[nockieboy]

There's some pieces missing from this puzzle. 

Base memory addresses:
Destination and/or source base memory addresses need to be set somehow.  They're not going to be set by the geometry_xy_plotter, they're going to be set by something else - the Z80_bridge, perhaps?  The screen memory being written to could be anywhere in the GPU RAM and specified by software via the MAGGIE/hardware registers, right?

So the two 24-bit address registers for source/destination base addresses will need to be set via an additional command/input route?

Output bus format:
In terms of the output (the 36-bit pixel_cmd bus), is there any need to deviate from the existing draw_cmd packing specification?  Instead of two 12-bit x/y coordinates, they'll be replaced by a 24-bit address, but the colour and aux_cmd values can remain - the need for the colour value is obvious, but the aux_cmd value will tell the pixel_writer whether it's writing or reading.  Am I missing something? 

Code: [Select]

module pixel_address_generator (

    // inputs
    input wire         clk,              // System clock
    input wire         reset,            // Force reset
   
    input wire         draw_cmd_rdy,     // Pulsed HIGH when data on draw_cmd[15:0] is valid
    input wire [35:0]  draw_cmd,         // Bits [35:32] hold AUX function number 0-15:

    // outputs
    output wire        pixel_cmd_rdy,
    output wire [35:0] pixel_cmd,
    output wire [3:0]  memory_w/r_bit_size,
    output wire [3:0]  memory_w/r_bit_offset
   
);

// 3     3 3             2 2               1     1 1     0 0             0
// 5     2 1             4 3               5     2 1     8 7             0
// |     | |             | |               |     | |     | |             |
// 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
// |AUX01| |----COLOUR---| |-----Y COORDINATE----| |-----X COORDINATE----|
// |AUX02| |----COLOUR---| |-----Y COORDINATE----| |-----X COORDINATE----|
// |AUX03|                 |-----Y COORDINATE----| |-----X COORDINATE----|
// |AUX04|                 |-----Y COORDINATE----| |-----X COORDINATE----|
// ...
// |AUX06|                 |-----Y COORDINATE----| |-----X COORDINATE----|
// |AUX07| |-ALPHA BLEND-| |-------------24-bit RGB COLOUR---------------|
// ...
// |AUX10| |-TRANSP MASK-| |RGB 24-bit MASK COLOUR OR 3 ADD TRANS COLOURS|
// |AUX11| |-TRANSP MASK-| |RGB 24-bit MASK COLOUR OR 3 ADD TRANS COLOURS|
// |AUX12|                                 |---DEST RASTER IMAGE WIDTH---|
// |AUX13|                                 |--SOURCE RASTER IMAGE WIDTH--|
// |AUX14| |BITPLANE MODE| |-------DESTINATION BASE MEMORY ADDRESS-------|
// |AUX15| |BITPLANE MODE| |----------SOURCE BASE MEMORY ADDRESS---------|

wire [3:0]  aux_cmd_in = draw_cmd[35:32];
wire [11:0]          x = draw_cmd[11:0];
wire [11:0]          y = draw_cmd[23:12];

reg  [23:0] complete_address         = 24'b0;
reg  [23:0] destination_base_address = 24'b0;
reg  [23:0] source_base_address      = 24'b0;

//assign pixel_cmd = complete_address[19:0] >> ( the correct amount of 16-bit words per pixel)

always @(posedge clk or posedge reset) begin

    if ( draw_cmd_rdy ) begin

case (aux_cmd_in) begin
   
4'd01 : begin

// write pixel with colour, x & y - generate address for pixel
complete_address[23:0] <= {destination_base_address[23:1], 1'b0} + ( y * scr_width + x );

end

4'd02 :

4'd03 :

4'd04 :

4'd06 : begin

// write pixel with x & y

end

endcase

    end

end // always @ posedge clk or reset

endmodule


[BrianHG]

The geometry_xy_plotter tells the pixel_address_generator what it is.  It is stored in the pixel address generator.

Take a look at lines 143 through 157 in the geometry_xy_plotter.  It does send out an aux command with an address.

Things sent to the pixel writer:
You do need to send an address to the pixel_writer with what 8 bit color, which bits within the 16bit word are affected, if its a read or a write pixel, will the write pixel use a transparent color mask.  Or, if the written color should be what's in the read pixel color buffer multiplied/mixed by a source color.

There is also the reset read pixel collision pixel counter and write collision pixel counter.

[nockieboy]

Things sent to the pixel writer:
You do need to send an address to the pixel_writer with what 8 bit color, which bits within the 16bit word are affected, if its a read or a write pixel, will the write pixel use a transparent color mask.  Or, if the written color should be what's in the read pixel color buffer multiplied/mixed by a source color.

Okay, I've had a little time this morning to do some work on this (but time is getting more scarce at the moment ), have attached latest code.  I've added cases for setting raster width and base addresses and am using those registers in the memory calculations for AUX 1 and 6 now.  I've sketched out a rough idea of what pixel_cmd's composition should look like and am assigning values to it for the two AUX commands you asked me to start with.

As far as the pixel_cmd and its constituent bits, what exactly do you mean by "which bits within the 16bit word are affected"?  I've taken that to mean whether the high or low byte is being written to, but am not sure that's what you mean and am not sure how to handle this. 

There is also the reset read pixel collision pixel counter and write collision pixel counter.

Where are these counters going to live?  In the pixel_writer?

[BrianHG]

Actually, you are doing good.

Now, you cannot have:
pixel_cmd_rdy    <= 1'b1;
for a wire, only a reg...

To make your life easier, look at my fifo 3 word and switch to system verilog logic syntax and let the compiler auto-select whether your logic names are regs or wires.

This includes 'always_ff' for the clocked regs & before that, the 'always_comb' for the combinational logic which replaces the assigns with the same lines, but, no 'assign' needed.

Please be more defined about what the ' 4'd06 : begin   // write pixel with x & y, no colour' means.
Why, what's different?

Also:
{destination_base_address[23:1], 1'b0} + ( y * dest_rast_width[15:0] + x )

Can create an odd memory address and the entire line does not reflect the proper read/write address if you are in 16bits per pixel mode, 4 bits per pixel mode, 2 bits per pixel mode, or, 1 bit per pixel mode.  It only reflects the proper address when in 8 bit per pixel mode, but with the LSB swinging as if we are addressing 8 bits.

Maybe you should make a temporary logic name in the combinational section like:
dest_base_address_offset = ( (y * dest_rast_width[15:0] + x ) << 1 ) >> CORRECT NUMBER OF BITS base on bits per pixel  )
and in the:
            4'd01 : begin   // write pixel with colour, x & y
           
                pixel_cmd[19:0]     <= (destination_base_address +  dest_base_address_offset) and shave off the LSB
                pixel_cmd[23:20]   <= which bit offset should be edited

Also:   4'd14 : dest_base_address[23:0] <= { draw_cmd[23:1], 1'b0 };
Do not shave off the LSB here.

Aslo:

            4'd01 : begin   // write pixel with colour, x & y
           
                pixel_cmd[23:0]  <= {destination_base_address[23:1], 1'b0} + ( y * dest_rast_width[15:0] + x ); // generate address for the pixel
                pixel_cmd[31:24] <= draw_cmd[31:24];                                                            // include colour information
                pixel_cmd[35]    <= 1'b1;                                                                       // WRITE command
                pixel_cmd[34:32] <= 3'b0;                                                                       // No transparency, no R/M/W, affected bits unknown
                pixel_cmd_rdy    <= 1'b1;

Maybe you should combine these 2 lines into a ' pixel_cmd[35:32] <= 4'd## '
Also, you haven't created a difference between the pixel write command # 4'd01 and # 4'd06.

Usually what we would do here is create a localparams:
(these are examples, you may switch them the way you like)
cmd_out_write_pixel_color = 4'b1000
cmd_out_write_pixel_color_transparent = 4'b1001

and in the transmit command section:
                pixel_cmd[35:32] <= cmd_out_write_pixel_color ;

This way in the next pixel writer module, you can import the localparams and switch the
cmd_out_ to cmd_in_
and use them for you case statement there.

Just like you should right now go into the geometry_xy_plotter, copy and paste it's localparams:

Code: [Select]

localparam CMD_OUT_NOP           = 0;
localparam CMD_OUT_PXWRI         = 1;
localparam CMD_OUT_PXWRI_M       = 2;
localparam CMD_OUT_PXPASTE       = 3;
localparam CMD_OUT_PXPASTE_M     = 4;

localparam CMD_OUT_PXCOPY        = 6;
localparam CMD_OUT_SETARGB       = 7;

localparam CMD_OUT_RST_PXWRI_M   = 10;
localparam CMD_OUT_RST_PXPASTE_M = 11;
localparam CMD_OUT_DSTRWDTH      = 12;
localparam CMD_OUT_SRCRWDTH      = 13;
localparam CMD_OUT_DSTMADDR      = 14;
localparam CMD_OUT_SRCMADDR      = 15;
replace the 'CMD_OUT_' with 'CMD_IN_' and use them in your case statements instead of the completely nondescript:
            4'd15 :

So far so good.

Next, you will need to decide how to pass the information like which bits in a 16 bit word are to be edited.  the grand total of 32 bits for address and color are just enough since we have only 20 bits addressing for maggie.  Lets see how you progress.

Separately, you should also need to tell the pixel writer how many bits are in a pixel as this information is needed as well.

[BrianHG]

I might not be available for August.  Better get a move on...

[nockieboy]

To make your life easier, look at my fifo 3 word and switch to system verilog logic syntax and let the compiler auto-select whether your logic names are regs or wires.

Okaaay.. I'm not familiar with the SystemVerilog syntax (not that it's much different), so I'm hoping I don't introduce more errors as a result.

Please be more defined about what the ' 4'd06 : begin   // write pixel with x & y, no colour' means.
Why, what's different?

There's no colour data?

Also:
{destination_base_address[23:1], 1'b0} + ( y * dest_rast_width[15:0] + x )

Can create an odd memory address and the entire line does not reflect the proper read/write address if you are in 16bits per pixel mode, 4 bits per pixel mode, 2 bits per pixel mode, or, 1 bit per pixel mode.  It only reflects the proper address when in 8 bit per pixel mode, but with the LSB swinging as if we are addressing 8 bits.

Maybe you should make a temporary logic name in the combinational section like:
dest_base_address_offset = ( (y * dest_rast_width[15:0] + x ) << 1 ) >> CORRECT NUMBER OF BITS base on bits per pixel  )

Okay, done that, but I'm unsure about how the 'correct number of bits' relates to bits per pixel.  Firstly, how is bits-per-pixel getting into the module?  Is it going to be set by the geometry_xy_plotter, or the Z80_bridge, or something else?

Also:   4'd14 : dest_base_address[23:0] <= { draw_cmd[23:1], 1'b0 };
Do not shave off the LSB here.

Should I also not be shaving off the LSB for 4'd15 as well, or is the source address being treated differently?

Aslo:

            4'd01 : begin   // write pixel with colour, x & y
           
                pixel_cmd[23:0]  <= {destination_base_address[23:1], 1'b0} + ( y * dest_rast_width[15:0] + x ); // generate address for the pixel
                pixel_cmd[31:24] <= draw_cmd[31:24];                                                            // include colour information
                pixel_cmd[35]    <= 1'b1;                                                                       // WRITE command
                pixel_cmd[34:32] <= 3'b0;                                                                       // No transparency, no R/M/W, affected bits unknown
                pixel_cmd_rdy    <= 1'b1;

Maybe you should combine these 2 lines into a ' pixel_cmd[35:32] <= 4'd## '
Also, you haven't created a difference between the pixel write command # 4'd01 and # 4'd06.

Ah. I've added a bit to the CMD part of pixel_cmd to allow for colour/no-colour (the 36th bit).

Hang on - have I misunderstood the TRANSPARENCY bit?  Is that what I should be using for 4'd06? 

So far so good.

Next, you will need to decide how to pass the information like which bits in a 16 bit word are to be edited.  the grand total of 32 bits for address and color are just enough since we have only 20 bits addressing for maggie.  Lets see how you progress.

Okay, well I think I've done that - with the bits saved from dropping the memory address from 24- to 20-bits, I've included 4 bits (unimaginatively called 'BIT' in the code comments for the pixel_cmd format) to identify exactly which bit in the 16-bit word needs modifying.

Where I really need some help is with bit_mode.  How does that feed into dest_base_address_offset etc?  Is it literally just a bit-shift right as you've indicated in the example for dest_base_address_offset?

Separately, you should also need to tell the pixel writer how many bits are in a pixel as this information is needed as well.

Yes, I've asked about this above.  Is this going to feed in from the video generator, or somewhere else?

[nockieboy]

I might not be available for August.  Better get a move on...

Going as fast as I can, but I'm BAF this week (and probably next as well). 

[BrianHG]

To make your life easier, look at my fifo 3 word and switch to system verilog logic syntax and let the compiler auto-select whether your logic names are regs or wires.

Okaaay.. I'm not familiar with the SystemVerilog syntax (not that it's much different), so I'm hoping I don't introduce more errors as a result.

Please be more defined about what the ' 4'd06 : begin   // write pixel with x & y, no colour' means.
Why, what's different?

There's no colour data?

Perhaps you didn't catch my drift.
Does function #6 even write a pixel?
I thought it is a copy IE: read pixel command.

Also:
{destination_base_address[23:1], 1'b0} + ( y * dest_rast_width[15:0] + x )

Can create an odd memory address and the entire line does not reflect the proper read/write address if you are in 16bits per pixel mode, 4 bits per pixel mode, 2 bits per pixel mode, or, 1 bit per pixel mode.  It only reflects the proper address when in 8 bit per pixel mode, but with the LSB swinging as if we are addressing 8 bits.

Maybe you should make a temporary logic name in the combinational section like:
dest_base_address_offset = ( (y * dest_rast_width[15:0] + x ) << 1 ) >> CORRECT NUMBER OF BITS base on bits per pixel  )

Okay, done that, but I'm unsure about how the 'correct number of bits' relates to bits per pixel.  Firstly, how is bits-per-pixel getting into the module?  Is it going to be set by the geometry_xy_plotter, or the Z80_bridge, or something else?

Looking inside the geometry unit, in the transmitted commands section, #14 and #15, after the address:

Code: [Select]

    //  AUX=14 : Set destination mem address,             : 31:24 bitplane mode : 23:0 hold destination base memory addres for write pixel
    //  AUX=15 : Set source mem address,                  : 31:24 bitplane mode : 23:0 hold the source base memory address for read source pixel
The 'bitplane mode' will contain the number of bits per pixel.
Remember, the copy pixel and write pixel may have 2 different bitplane modes.  This way if you want to print a 2 color font, IE 1 bit per pixel, onto a 16 color graphics screen, IE 4 bits per pixel, the address generator will send both individual values to the pixel writer/reader so it may perform the translation in real time.  Without this, you would have to store a 16 color font for your 16 color screen which takes more ram and also wont allow you to chose individual character colors as the font itself would already be colorized.

Also:   4'd14 : dest_base_address[23:0] <= { draw_cmd[23:1], 1'b0 };
Do not shave off the LSB here.

Should I also not be shaving off the LSB for 4'd15 as well, or is the source address being treated differently?

Code: [Select]

I guess what I was trying to say is that the shaving should only be done right at the last step before output.  Technically, since we are calculating a 16 bit offset for the read/write pointer, this should automatically be done by the math if the formula is right.


Aslo:

            4'd01 : begin   // write pixel with colour, x & y
           
                pixel_cmd[23:0]  <= {destination_base_address[23:1], 1'b0} + ( y * dest_rast_width[15:0] + x ); // generate address for the pixel
                pixel_cmd[31:24] <= draw_cmd[31:24];                                                            // include colour information
                pixel_cmd[35]    <= 1'b1;                                                                       // WRITE command
                pixel_cmd[34:32] <= 3'b0;                                                                       // No transparency, no R/M/W, affected bits unknown
                pixel_cmd_rdy    <= 1'b1;

Maybe you should combine these 2 lines into a ' pixel_cmd[35:32] <= 4'd## '
Also, you haven't created a difference between the pixel write command # 4'd01 and # 4'd06.

Ah. I've added a bit to the CMD part of pixel_cmd to allow for colour/no-colour (the 36th bit).

Hang on - have I misunderstood the TRANSPARENCY bit?  Is that what I should be using for 4'd06? 

So far so good.

Next, you will need to decide how to pass the information like which bits in a 16 bit word are to be edited.  the grand total of 32 bits for address and color are just enough since we have only 20 bits addressing for maggie.  Lets see how you progress.

Okay, well I think I've done that - with the bits saved from dropping the memory address from 24- to 20-bits, I've included 4 bits (unimaginatively called 'BIT' in the code comments for the pixel_cmd format) to identify exactly which bit in the 16-bit word needs modifying.

Where I really need some help is with bit_mode.  How does that feed into dest_base_address_offset etc?  Is it literally just a bit-shift right as you've indicated in the example for dest_base_address_offset?

Separately, you should also need to tell the pixel writer how many bits are in a pixel as this information is needed as well.

Yes, I've asked about this above.  Is this going to feed in from the video generator, or somewhere else?

Ok, about the bit selection, and the missing bit width.
The -

Code: [Select]

    //  AUX=14 : Set destination mem address,             : 31:24 bitplane mode : 23:0 hold destination base memory addres for write pixel
    //  AUX=15 : Set source mem address,                  : 31:24 bitplane mode : 23:0 hold the source base memory address for read source pixel
- bitplane mode tells the address generator how many bits there are in a pixel.  The address generator needs to tell the pixel writer that number of bits in a pixel.  It also needs to use this information to calculate the right memory address as the X coordinate will now change how much the address increases, but also, depending on the pixel width and where the X coordinate lands, the pixel writer also needs that information as well so it may edit the correct bits within the word.  IE, the pixel writer knowing the bits per pixel set by the 'bitplane mode' is half of what it needs.  It also needs where inside the 16 bits the X coordinate landed on so it may edit the right parts of the 16 bit word.

This second piece of information in effect is the correct lower 0-3 bits of the X coordinates.  Also, your output bus isn't large enough for this added 4 bits, so a total of 36 bits needs another 4 bits of data.

BITS:
20 - (A) address
  4 - (B) pixel width bits
  4 - (C) sub-pixel location within a 16 bit word
  8 - (D) color function
  4 - (E) function to perform IE AUX command
----------------------------------------------------------
40 bits total.

Now, since (A&C&D&E) are needed to read and write a pixel while (B) can be sent to the pixel_reader/writer as it's own function, you can cheat and still use a 36bit bus, but, when we add a fifo between the geo_address_generator and the geo_pixel_reader_writer, that will be 1 additional word in the fifo which will occasionally need to be sent.  With a 40 bit bus, we eliminate that 1 word in out 3 word cache since the fifo will now be 40bit instead of 36 bit and that info will be sent in parallel with every read or write command.

[nockieboy]

Perhaps you didn't catch my drift.
Does function #6 even write a pixel?
I thought it is a copy IE: read pixel command.

Uh... of course it is. 

Ok, about the bit selection, and the missing bit width.
The -

Code: [Select]

    //  AUX=14 : Set destination mem address,             : 31:24 bitplane mode : 23:0 hold destination base memory addres for write pixel
    //  AUX=15 : Set source mem address,                  : 31:24 bitplane mode : 23:0 hold the source base memory address for read source pixel
- bitplane mode tells the address generator how many bits there are in a pixel.  The address generator needs to tell the pixel writer that number of bits in a pixel.  It also needs to use this information to calculate the right memory address as the X coordinate will now change how much the address increases, but also, depending on the pixel width and where the X coordinate lands, the pixel writer also needs that information as well so it may edit the correct bits within the word.  IE, the pixel writer knowing the bits per pixel set by the 'bitplane mode' is half of what it needs.  It also needs where inside the 16 bits the X coordinate landed on so it may edit the right parts of the 16 bit word.

This second piece of information in effect is the correct lower 0-3 bits of the X coordinates.

There's probably a blindingly obvious solution to calculating the address based on the X,Y position, bitplane mode and pixel width involving a simple bit-shift, but I'm feeling like the guy in the picture above trying to work it out.

As far as the pixel_cmd bus goes, I've widened it to 40-bits and included the changes from your last post, made a few changes to the code accordingly, but am hanging on the solution to the address calculations based off of the supplied parameters.

The address generator needs:

• X & Y coordinates
• destination (or source?) raster width
• bits per pixel

From that it should be able to spit out:

• A valid memory address for the pixel writer targeting a byte or word depending on screen mode
• A valid bit in that byte or word, based on the bits per pixel screen mode

Right?

[BrianHG]

I gave you the correct address calculation in Reply #1354.  Read the line with blue text and the next few lines after.

The Z80 tells the geometry xy plotter which base address, raster pixel width and bits per pixel will be used when writing/reading pixels.  You need to set that since you can have multiple screens with different color depths and widths.  So, those figures need to change when plotting / addressing a new screen.

The geometry passes these settings into the pixel address generator when it sends the:

Code: [Select]

            CMD_IN_DSTRWDTH : dest_rast_width[15:0]   <= draw_cmd[15:0];    // set destination raster image width
            CMD_IN_SRCRWDTH : srce_rast_width[15:0]   <= draw_cmd[15:0];    // set source raster image width
            CMD_IN_DSTMADDR : dest_base_address[23:0] <= draw_cmd[23:0];    // set destination base memory address
            CMD_IN_SRCMADDR : srce_base_address[23:0] <= draw_cmd[23:0];    // set source base memory address )

Now, with the last 2, you only forgot to latch/retain the 4 bit setting srce_ and desc_ bits per pixel.

Now, like you said, when a write or read pixel is received from the geometry plotter, the pixel address generator takes the stored proper dest_xxxx or srce_xxxx depending on a pixel read or write, and sends it out to the pixel writer if it is not busy.

The only weird commands sent to the pixel_write are these:

Code: [Select]

localparam CMD_IN_SETARGB       = 7;
localparam CMD_IN_RST_PXWRI_M   = 10;
localparam CMD_IN_RST_PXPASTE_M = 11;
The setargb just passes the first 32bits input to the first 32 bits in the output.
The RST_xxx just sends the command, the data bits on the output are ignored.

Your almost done.  Setup a simulation test bench, fill in the final address math, compile and make sure your FMAX passes 125MHz and we should be ready to do the pixel writer.  This one is a little tricky if you want to make it extra fast and efficient, but once working, you are ready to draw hardware accelerated lines, boxes & filled boxes.  Your will next need to add triangles and filled triangles referencing the work in geo.bas.

Also, in the geometry xy plotter, you will finally add a simple rectangular box copy command which will copy a rectangle from the video source video port to the destination port.  All line drawing functions, not fills, should also be able to drive the destination coordinates of the copy function and use the copy source as a paint brush so you may, as an example, hardware accelerate draw a fat circular brush for the line function when pixel writing using the transparency feature.

[nockieboy]

I gave you the correct address calculation in Reply #1354.  Read the line with blue text and the next few lines after.

So this is correct, then?

Code: [Select]

dest_base_address_offset = ( ( y * dest_rast_width[15:0] + x ) << 1 ) >> bits_per_pixel[3:0] );
...and...

Code: [Select]

pixel_cmd[19:0]  <= { destination_base_address[19:0] + dest_base_address_offset, 1'b0 };  // generate address for the pixel
?

The Z80 tells the geometry xy plotter which base address, raster pixel width and bits per pixel will be used when writing/reading pixels.

The geometry passes these settings into the pixel address generator when it sends the:

Code: [Select]

            CMD_IN_DSTRWDTH : dest_rast_width[15:0]   <= draw_cmd[15:0];    // set destination raster image width
            CMD_IN_SRCRWDTH : srce_rast_width[15:0]   <= draw_cmd[15:0];    // set source raster image width
            CMD_IN_DSTMADDR : dest_base_address[23:0] <= draw_cmd[23:0];    // set destination base memory address
            CMD_IN_SRCMADDR : srce_base_address[23:0] <= draw_cmd[23:0];    // set source base memory address )

Now, with the last 2, you only forgot to latch/retain the 4 bit setting srce_ and desc_ bits per pixel.

Ah yes, I missed that.  Okay, I've created a register to store the screen mode (bits_per_pixel).  However, I'm a little unsure that I've done it properly.  bits_per_pixel is only 4 bits, but bit_mode (where bits_per_pixel is latched from) is 8 bits as the plotter passes an 8-bit field for BITPLANE-MODE - I've set bits_per_pixel from the bottom 4-bits of the BITPLANE-MODE field.  Is that okay?

I've been trying to find where this BITPLANE-MODE 8-bit field in draw_cmd is set in the geometry_xy_plotter.sv, and aside from not being able to find it, I think I've found an error in the code as well.  Bearing in mind that CMD_OUT_DSTMADDR is defined as a localparam on line 42, it's later referenced as a bus on line 144?

Code: [Select]

Line 42 : localparam CMD_OUT_DSTMADDR      = 14;
Line 144: draw_cmd_func        <= CMD_OUT_DSTMADDR[3:0];   // sets the output function

Same issue with CMD_OUT_SRCMADDR on line 152. 

The only weird commands sent to the pixel_write are these:

Code: [Select]

localparam CMD_IN_SETARGB       = 7;
localparam CMD_IN_RST_PXWRI_M   = 10;
localparam CMD_IN_RST_PXPASTE_M = 11;
The setargb just passes the first 32bits input to the first 32 bits in the output.
The RST_xxx just sends the command, the data bits on the output are ignored.

Okay, I've added the case statement code for those commands now.  Hopefully all correct.

Your almost done.  Setup a simulation test bench, fill in the final address math, compile and make sure your FMAX passes 125MHz and we should be ready to do the pixel writer.  This one is a little tricky if you want to make it extra fast and efficient, but once working, you are ready to draw hardware accelerated lines, boxes & filled boxes.  Your will next need to add triangles and filled triangles referencing the work in geo.bas.

Just need to sort these last niggling questions above, and I can't shake the feeling that the 'final address math' isn't complete - I'll be a lot happier once you've given me the official thumbs-up for what's in the attached code.

[BrianHG]

Got thing a little backwards:

Code: [Select]

            CMD_IN_DSTMADDR : begin
                dest_base_address[23:0] <= draw_cmd[23:0];    // set destination base memory address
                bits_per_pixel[3:0]        <= bit_mode[3:0];     // set screen mode (bits per pixel)
            end
           
            CMD_IN_SRCMADDR : begin
                srce_base_address[23:0] <= draw_cmd[23:0];    // set source base memory address (even addresses only?)
                bits_per_pixel[3:0]     <= bit_mode[3:0];     // set screen mode (bits per pixel)
            end

Ok, I see where 'draw_cmd[23:0]' is coming from the command input, but, where is 'bit_mode[3:0]' coming from?
Also, if you are reading a srce 1 bit font to write onto an 8 bit dest screen, how will your pixel read(copy) and pixel write command switch between the 2 when they go back and forth pixel by pixel if you are erasing/overwriting that same ' bits_per_pixel[3:0]' register?

Next :

Code: [Select]

            CMD_IN_PXCOPY : begin   // read pixel with x & y
                pixel_cmd[19:0]  <= { destination_base_address[19:0] + dest_base_address_offset, 1'b0 };  // generate address for the pixel
                pixel_cmd[23:20] <= target_bit;                                                           // which bit to read from the addressed byte
                pixel_cmd[27:24] <= bits_per_pixel[3:0];                                                  // set bits per pixel for current screen mode
                pixel_cmd[35:28] <= 8'b0;                                                                 // no colour information
                pixel_cmd[39:36] <= CMD_OUT_READ_PIXEL;                                                   // NO COLOUR, READ, NO TRANS, NO R/M/W
                pixel_cmd_rdy    <= 1'b1;
            end
Why are you transmitting the pixel writing destination base address (plus extras) when you are calling the (copy) read source pixel command?

This line (this my be partially my fault):

Code: [Select]

    dest_base_address_offset = ( ( y * dest_rast_width[15:0] + x ) << 1 ) >> bits_per_pixel[3:0] );
Now, if we have 1 bit per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 2 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 4 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 8 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 16 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.

Now, can we even make a 4 bit number 1 through 16?

You will see that your shift wont give you the right results unless you will be placing a custom figure for 'bits_per_pixel[3:0]' calculated by the Z80.  Perhaps look at maggies bit/pixel setting and try to match it's figure so that the Z80 may just copy the figure of the screen setting if you like.

[nockieboy]

Got thing a little backwards:

Code: [Select]

            CMD_IN_DSTMADDR : begin
                dest_base_address[23:0] <= draw_cmd[23:0];    // set destination base memory address
                bits_per_pixel[3:0]        <= bit_mode[3:0];     // set screen mode (bits per pixel)
            end
           
            CMD_IN_SRCMADDR : begin
                srce_base_address[23:0] <= draw_cmd[23:0];    // set source base memory address (even addresses only?)
                bits_per_pixel[3:0]     <= bit_mode[3:0];     // set screen mode (bits per pixel)
            end


Backwards? How so?

Ok, I see where 'draw_cmd[23:0]' is coming from the command input, but, where is 'bit_mode[3:0]' coming from?

bit_mode[3:0] is set here:

Code: [Select]

Line 93:     bit_mode[7:0]   = draw_cmd[31:24];  // number of bits per pixel

This relates to a question I asked in my previous post - there seems to be 8 bits set aside in draw_cmd for bitplane_mode, but only 4 bits for passing it on to the pixel writer, so I chose the bottom 4 bits?

Also, if you are reading a srce 1 bit font to write onto an 8 bit dest screen, how will your pixel read(copy) and pixel write command switch between the 2 when they go back and forth pixel by pixel if you are erasing/overwriting that same ' bits_per_pixel[3:0]' register?

So I needed two bits_per_pixel registers, one for source and one for destination?  Have added them in now.

Why are you transmitting the pixel writing destination base address (plus extras) when you are calling the (copy) read source pixel command?

Fixed. 

This line (this my be partially my fault):

Code: [Select]

    dest_base_address_offset = ( ( y * dest_rast_width[15:0] + x ) << 1 ) >> bits_per_pixel[3:0] );
Now, if we have 1 bit per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 2 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 4 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 8 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.
Now, if we have 16 bits per pixel, and each memory address is 16 bits wide, how much do we divide the x coordinate.

Now, can we even make a 4 bit number 1 through 16?

You will see that your shift wont give you the right results unless you will be placing a custom figure for 'bits_per_pixel[3:0]' calculated by the Z80.  Perhaps look at maggies bit/pixel setting and try to match it's figure so that the Z80 may just copy the figure of the screen setting if you like.

  I have no idea how the address calculation maths is working here anyway, so I just went with what you said.  I'll need to go away and think this through, so will come back to it over the weekend (hopefully!) 

[BrianHG]

Code: [Select]

localparam CMD_OUT_WRITE_PIXEL_COLOUR   = 5'b11000;
localparam CMD_OUT_READ_PIXEL           = 5'b00000;
5 bits?

Code: [Select]

    dest_base_address_offset = ( ( y * dest_rast_width[15:0] + x ) << 1 ) >> dest_bits_per_pixel[3:0] );
    srce_base_address_offset = ( ( y * srce_rast_width[15:0] + x ) << 1 ) >> srce_bits_per_pixel[3:0] );

Code: [Select]

                pixel_cmd[19:0]  <= { destination_base_address[19:0] + dest_base_address_offset, 1'b0 };  // generate address for the pixel
You need to work out and learn how you access pixels and what this code is doing and decide if it does what you want it to do.

Now, I've said that every read or written byte by the GEO port reads and writes 16 bits at once ignoring the address LSB.

I will repeat.  In 8 bit color mode, each pixel is 1 byte, or 2 pixels consume one 16 bit word.  In 16 bit mode, 1 pixel is 2 bytes, or when addressing a 16 bit word, 1 pixel = one 16 bit word.  In 4 bit mode, 2 pixels make up 1 byte, or, 4 pixels make up one 16 bit word.  Ect.

You have a base address.  This address point to the first byte in the display graphics memory.

You also have a X&Y coordinate and a rast_width which defines how many pixels to jump for every Y coordinate position.  Now when calculating the new memory position to be addressed, remember you want to know how much you need to add to the base address to get to the right pixel.

You also need a means of telling the address generator how many bits per pixel (you have set in MAGGIE) so it may do the mathematics properly, this is the bits_per_pixel[] register setting.

Begin with thinking that you only have 8 bit pixels with 8 bit memory.  What would that math look like?
Next, how would you change that calculation if you had a 4 bit pixel where every 2 pixels take up 1 byte?

Again, think this through carefully and I'm sure you can solve the problem.


Next:

Code: [Select]

            CMD_IN_PXWRI_M :
           
            CMD_IN_PXPASTE :
           
            CMD_IN_PXPASTE_M :

These are identical to the 'CMD_IN_PXWRI' except they send out a different function to the pixel writer so it knows to enable the feature when generating the pixel to be written.

Next, the :

Code: [Select]

            CMD_IN_SETARGB       : pixel_cmd[31:0]    <= draw_cmd[31:0];    // pass through first 32-bits of input to output ( Alpha Blend and 24-bit RGB colour data)
           
            CMD_IN_RST_PXWRI_M   : pixel_cmd[39:36]   <= draw_cmd[35:32];   // pass through command only
           
            CMD_IN_RST_PXPASTE_M : pixel_cmd[39:36]   <= draw_cmd[35:32];   // pass through command only

If you never send out a:

Code: [Select]

                pixel_cmd_rdy    <= 1'b1;The pixel writer will never know it was being sent a command to take action.

After all is said and done, you also need to add a:

Code: [Select]

    input logic draw_busy,        // HIGH when pixel writer is busy, so address generator will pause before sending any new pixels
And a reset as well.

Let's see a good test simulation so we can go onto the pixel writer.

[nockieboy]

Code: [Select]

localparam CMD_OUT_WRITE_PIXEL_COLOUR   = 5'b11000;
localparam CMD_OUT_READ_PIXEL           = 5'b00000;
5 bits?

Well spotted. 

You also need a means of telling the address generator how many bits per pixel (you have set in MAGGIE) so it may do the mathematics properly, this is the bits_per_pixel[] register setting.

I'd tacked this onto the CMD_IN_DSTMADDR and CMD_IN_SRCMADDR functions - so dest/src_bits_per_pixel is set by whatever is in the bit_mode bits in the draw_cmd.  However, there's nothing in the geo module to pass the bitmode value and it doesn't seem logical that it would know the screen mode anyway.  You've mentioned getting bits_per_pixel from the MAGGIE - but which one?  How would the address generator know which MAGGIE to read?

Begin with thinking that you only have 8 bit pixels with 8 bit memory.  What would that math look like?

Next, how would you change that calculation if you had a 4 bit pixel where every 2 pixels take up 1 byte?

Like this?

Code: [Select]

dest_base_address_offset = y * dest_rast_width[15:0] + ( x << dest_bits_per_pixel[3:0] );
srce_base_address_offset = y * srce_rast_width[15:0] + ( x << srce_bits_per_pixel[3:0] );

Next:

Code: [Select]

            CMD_IN_PXWRI_M :
           
            CMD_IN_PXPASTE :
           
            CMD_IN_PXPASTE_M :

These are identical to the 'CMD_IN_PXWRI' except they send out a different function to the pixel writer so it knows to enable the feature when generating the pixel to be written.

Okay, I've done CMD_IN_PXWRI_M (I think), but how are PXPASTE and PXPASTE_M any different to PXWRI and PXWRI_M?

After all is said and done, you also need to add a:

Code: [Select]

    input logic draw_busy,        // HIGH when pixel writer is busy, so address generator will pause before sending any new pixels
And a reset as well.

Okay, all should be present and correct. Latest code attached. I just need clarification on where the address generator is getting the bits_per_pixel value from.

[BrianHG]

Code: [Select]

localparam CMD_OUT_WRITE_PIXEL_COLOUR   = 5'b11000;
localparam CMD_OUT_READ_PIXEL           = 5'b00000;
5 bits?

Well spotted. 

You also need a means of telling the address generator how many bits per pixel (you have set in MAGGIE) so it may do the mathematics properly, this is the bits_per_pixel[] register setting.

I'd tacked this onto the CMD_IN_DSTMADDR and CMD_IN_SRCMADDR functions - so dest/src_bits_per_pixel is set by whatever is in the bit_mode bits in the draw_cmd.  However, there's nothing in the geo module to pass the bitmode value and it doesn't seem logical that it would know the screen mode anyway.  You've mentioned getting bits_per_pixel from the MAGGIE - but which one?  How would the address generator know which MAGGIE to read?

You are correct, it was accidentally omitted in the geo plotter module.
In the geo plotter, take a look at functions 8'b011111?? and 8'b011110??.  They are kinda wrong.
The 'draw_cmd_data_color' which is set to 8'h00 should contain the 'command_data8' where you would place the bitplane mode you desire.

Also, commands 8'd115 through 8'd112 should be re-written like this so you may support screen widths beyond 4096 pixels, they will now support the 65535 pixel wide screen.  In fact, I would prefer moving the bitplane mode to these 4 commands.

Code: [Select]

                       
                8'd115 : begin  // set the number of bytes per horizontal line in the destination raster
                    draw_cmd_func        <= CMD_OUT_DSTRWDTH[3:0];   // sets the output function
                    draw_cmd_data_color  <= command_data8;                   // clears the unused function data
                    draw_cmd_data_word_Y <= y[2];                   // null
                    draw_cmd_data_word_X <= x[2];                    // sets the lower 12 bits of the destination address
                    draw_cmd_tx          <= 1'b1;                    // transmits the command
                end
                   
                8'd114 : begin  // set the number of bytes per horizontal line in the destination raster
                    draw_cmd_func        <= CMD_OUT_SRCRWDTH[3:0];   // sets the output function
                    draw_cmd_data_color  <= command_data8;                   // clears the unused function data
                    draw_cmd_data_word_Y <= y[2];                    // sets the lower 12 bits of the destination address
                    draw_cmd_data_word_X <= x[2];                   // null
                    draw_cmd_tx          <= 1'b1;                    // transmits the command
                end
                   
                8'd113 : begin  // set the number of bytes per horizontal line in the destination raster
                    draw_cmd_func        <= CMD_OUT_DSTRWDTH[3:0];   // sets the output function
                    draw_cmd_data_color  <= command_data8;                   // clears the unused function data
                    draw_cmd_data_word_Y <= y[3];                   // null
                    draw_cmd_data_word_X <= x[3];                    // sets the lower 12 bits of the destination address
                    draw_cmd_tx          <= 1'b1;                    // transmits the command
                end
                   
                8'd112 : begin  // set the number of bytes per horizontal line in the destination raster
                    draw_cmd_func        <= CMD_OUT_SRCRWDTH[3:0];   // sets the output function
                    draw_cmd_data_color  <= command_data8;                   // clears the unused function data
                    draw_cmd_data_word_Y <= y[3];                    // sets the lower 12 bits of the destination address
                    draw_cmd_data_word_X <= x[3];                   // null
                    draw_cmd_tx          <= 1'b1;                    // transmits the command
                end


Begin with thinking that you only have 8 bit pixels with 8 bit memory.  What would that math look like?

Next, how would you change that calculation if you had a 4 bit pixel where every 2 pixels take up 1 byte?

Like this?

Code: [Select]

dest_base_address_offset = y * dest_rast_width[15:0] + ( x << dest_bits_per_pixel[3:0] );
srce_base_address_offset = y * srce_rast_width[15:0] + ( x << srce_bits_per_pixel[3:0] );

Ok, the changes you made now makes the 'dest_rast_width[15:0]' define the screen width in bytes, not pixel width.  There is nothing wrong with this as long as you know what you are doing when setting the register.

Now for the 'x' if you set the 'dest_bits_per_pixel[3:0]' to it's top value of 15, you are shifting the 'X' value by 15 slots, or multiplying X by 32768.  This means that with an dest_bits_per_pixel or 15 and an X coordinate or 2, your already at 65536, more than the memory available to you.

Remember, the shifts either multiply by 1,2,4,8,16,32 or in the other direction divide by 1,2,4,6,16,32 with a value of 0,1,2,3,4,5.  The earlier version was more right as you need to divide down the 'X' value feeds the address as the number of pixels in a byte increase.

Next:

Code: [Select]

            CMD_IN_PXWRI_M :
           
            CMD_IN_PXPASTE :
           
            CMD_IN_PXPASTE_M :

These are identical to the 'CMD_IN_PXWRI' except they send out a different function to the pixel writer so it knows to enable the feature when generating the pixel to be written.

Okay, I've done CMD_IN_PXWRI_M (I think), but how are PXPASTE and PXPASTE_M any different to PXWRI and PXWRI_M?

No, the PXPASTE and PXPASTE_M arent any different.  Inside the pixel writer module, it will select which color or where it's getting it's color from to draw depending which type of pixel write/paste command it receives.

After all is said and done, you also need to add a:

Code: [Select]

    input logic draw_busy,        // HIGH when pixel writer is busy, so address generator will pause before sending any new pixels
And a reset as well.

Okay, all should be present and correct. Latest code attached. I just need clarification on where the address generator is getting the bits_per_pixel value from.

[nockieboy]

You are correct, it was accidentally omitted in the geo plotter module.
In the geo plotter, take a look at functions 8'b011111?? and 8'b011110??.  They are kinda wrong.
The 'draw_cmd_data_color' which is set to 8'h00 should contain the 'command_data8' where you would place the bitplane mode you desire.

Also, commands 8'd115 through 8'd112 should be re-written like this so you may support screen widths beyond 4096 pixels, they will now support the 65535 pixel wide screen.  In fact, I would prefer moving the bitplane mode to these 4 commands.

Okay, code updated as suggested.  Updated files attached at the bottom of this post.

Begin with thinking that you only have 8 bit pixels with 8 bit memory.  What would that math look like?

Next, how would you change that calculation if you had a 4 bit pixel where every 2 pixels take up 1 byte?

Like this?

Code: [Select]

dest_base_address_offset = y * dest_rast_width[15:0] + ( x << dest_bits_per_pixel[3:0] );
srce_base_address_offset = y * srce_rast_width[15:0] + ( x << srce_bits_per_pixel[3:0] );

Ok, the changes you made now makes the 'dest_rast_width[15:0]' define the screen width in bytes, not pixel width.  There is nothing wrong with this as long as you know what you are doing when setting the register.

   It does?

Now for the 'x' if you set the 'dest_bits_per_pixel[3:0]' to it's top value of 15, you are shifting the 'X' value by 15 slots, or multiplying X by 32768.  This means that with an dest_bits_per_pixel or 15 and an X coordinate or 2, your already at 65536, more than the memory available to you.

Remember, the shifts either multiply by 1,2,4,8,16,32 or in the other direction divide by 1,2,4,6,16,32 with a value of 0,1,2,3,4,5.  The earlier version was more right as you need to divide down the 'X' value feeds the address as the number of pixels in a byte increase.

Uh, well it seemed to work on paper.    I've reverted the changes until I can work this out.  I need the X/Y coordinates to resolve to an 16-bit memory address, with a 4-bit value indicating the specific bit at that address that we're writing/reading to/from?  But the target bit could be a byte, a word, or a variable number of bits depending on bits_per_pixel... 

[BrianHG]

Updates to geo plotter good.

Now for your formula:
dest_base_address_offset = y * dest_rast_width[15:0] + ( x << dest_bits_per_pixel[3:0] );

Ok, if we want 8 bits per pixel, say you make the ' dest_bits_per_pixel[3:0]  ' = 0.
This will sort of work.
If X=0, you want to write the top 8 bits of 16 bit word address 0.
If X=1, you want to write the bottom 8 bits of 16 bit word address 0.
If X=2, you want to write the top 8 bits of 16 bit word address 2.
If X=3, you want to write the bottom 8 bits of 16 bit word address 2.
It would appear you just need to shave off the bottom address bit and use a ' dest_bits_per_pixel[3:0]  ' setting of 0 to get this to work.

Now, if you have 4 bits per pixel, this means:
If X=0, you want to write the top 4 bits of 16 bit word address 0.
If X=1, you want to write bits 8-11 of 16 bit word address 0.
If X=2, you want to write bits 4-7 of 16 bit word address 0.
If X=3, you want to write the bottom 4 bits 16 bit word address 0.
If X=4, you want to write the top 4 bits of 16 bit word address 2.
If X=5, you want to write bits 8-11 of 16 bit word address 2.
If X=6, you want to write bits 4-7 of 16 bit word address 2.
If X=7, you want to write the bottom 4 bits 16 bit word address 2.
What's the value of 'dest_bits_per_pixel[3:0]' I need to make this happen?

Now, if you have 16 bits per pixel, this means:
If X=0, you want to write all 16 bits of 16 bit word address 0.
If X=1, you want to write all 16 bits of 16 bit word address 2.
If X=2, you want to write all 16 bits of 16 bit word address 4.
If X=3, you want to write all 16 bits of 16 bit word address 6.
What's the value of 'dest_bits_per_pixel[3:0]' I need to make this happen?


Now, if you have 2 bits per pixel, this means:
If X=0, you want to write the top 2 bits of 16 bit word address 0.
If X=1, you want to write bits 12-13 of 16 bit word address 0.
If X=2, you want to write bits 10-11 of 16 bit word address 0.
If X=3, you want to write bits 8-9 of 16 bit word address 0.
If X=4, you want to write bits 6-7 of 16 bit word address 0.
If X=5, you want to write bits 4-5 of 16 bit word address 0.
If X=6, you want to write bits 2-3 of 16 bit word address 0.
If X=7, you want to write the bottom 2 bits 16 bit word address 0.
If X=8, you want to write the top 2 bits 16 bit word address 2.
What's the value of 'dest_bits_per_pixel[3:0]' I need to make this happen?

[BrianHG]

Remember, 'dest_bits_per_pixel[3:0]' just tells the address generator how many pixels per word.

You will be sending the to the pixel writer with the 'dest_bits_per_pixel[3:0]' so that module will also know how many bits make up 1 pixel and you will also send the least significant bits of the 'X'  coordinate which will tell the pixel writer which part of the 16bit word will be written to.

[nockieboy]

How about this?  I've pasted the same process for each bpp setting, although the process is slightly simplified for 16 and 8 bpp there should be no reason why the same algorithm can't be used on them all.

• For 16 bpp, X should be <<1 with the remainder (0) moved into the 'target_bit' to target the appropriate nybble in the target byte, then the last bit (0) becomes the 'target_byte' within the addressed word and finally the last bit is zeroed.
• For 8 bpp,  X should be <<0 with the remainder (0) moved into the 'target_bit' to target the appropriate nybble in the target byte, then the last bit becomes the 'target_byte' within the addressed word and finally the last bit is zeroed.
• For 4 bpp,  X should be >>1 with the remainder moved into the 'target_bit' to target the appropriate nybble in the target byte, then the last bit becomes the 'target_byte' within the addressed word and finally the last bit is zeroed.
• For 2 bpp,  X should be >>2 with the remainder moved into the 'target_bit' to target the appropriate crumb in the target byte, then the last bit becomes the 'target_byte' within the addressed word and is zeroed.
• For 1 bpp,  X should be >>3 with the remainder moved into the 'target_bit' to target the appropriate bit in the target byte, then the last bit becomes the 'target_byte' within the addressed word and is zeroed.

However, target_bit refers to the addressed nybble, crumb or bit depending on the bits_per_pixel (bpp) setting, but doesn't correlate directly to the correct bit number:

• target_bit 0 = bit 7 in the targeted byte
• target_bit 1 = bit 6 in the targeted byte
• etc...

I guess that would need to be inverted?

Am I on the right track?  I have no idea how this would translate to a single line of HDL without using a CASE or IF conditional. 

[BrianHG]

Ok, now you are on to it.  Yes, the same argument can be use for all bitplane modes.

You can do it this way:
First do a << 1 to everything in brackets.
Then do a >> dest_bits_per_pixel[2:0] to that bracket.
Noting that this value isn't truly bits/pixel.

You cal always use a lookup table and convert dest_bits_per_pixel[3:0] into a proper 'shift' value if you like.
Something like a:
parameter int LUT_bits_to_shift[16]      = '{4,3,3,2,2,2,2,1,1,1,1,1,1,1,1,0};  // shift bits/pixel-1  0=1 bit, 1=2bit, 3=4bit, 7=8bit, 15-16bit.
Though this is more dumb from a programming stand point on the Z80 side.

Don't forget to include the Y*dest_rast_width[15:0] in the brackets so that the Y position is included in the math.

[nockieboy]

Okay, I'm using your lookup table suggestion as a simpler solution isn't jumping out at me:

Code: [Select]

parameter int LUT_bits_to_shift[16] = '{ 4,3,3,2,2,2,2,1,1,1,1,1,1,1,1,0 };  // shift bits/pixel-1  0=1 bit, 1=2bit, 3=4bit, 7=8bit, 15-16bit.

dest_base_address_offset = ( ( y * dest_rast_width[15:0] + x ) << 1 ) >> LUT_bits_to_shift[dest_bits_per_pixel[3:0]] );
srce_base_address_offset = ( ( y * srce_rast_width[15:0] + x ) << 1 ) >> LUT_bits_to_shift[srce_bits_per_pixel[3:0]] );

Updated code attached.

Now I just need to move bit 0 of the address_offset into target_byte, take the remainder and place that in target_bit, then zero the last bit of address_offset?

[BrianHG]

You got it, now, there is 1 last mistake:

Code: [Select]

pixel_cmd[19:0]  <= { destination_base_address[19:0] + dest_base_address_offset, 1'b0 };  // generate address for the pixel

I basically want address bit 0 cleared, not the whole thing shifted.
Better:

Code: [Select]

pixel_cmd[19:0]  <= (destination_base_address[19:0] + dest_base_address_offset) && 20'b11111111111111111110 ;  // generate address for the pixel

Do this for all of them.

Come to think of it, you could have done everything right shifted by 1, including the base address.  Then on this add line, left shift everything by 1.  But, no big difference.  The compiler would end up making the same gates anyways.

Also, what's the 'target_bit'?
In the comb section, you can make it:
target_bit = x[3:0];

Make sure this module works and now begin the pixel writer.

[BrianHG]

Specifications for the pixel writer coming in a few hours.  Be ready by making sure this module works.

The connection the geometry xy plotter unit and the address generator is direct.

The connection between the address generator and the pixel writer will have 1 of my 3 word fifo in between.

FIFO input side:
Data coming from the address generator should feed the fifo data input.
The fifo full output should feed both 'draw_busy' inputs of the geometry plotter and address generator.
The pixel_cmd_rdy output of the address generator should go through a gate:
(pixel_cmd_rdy && !fifo_full) -> this should feed the fifo's 'shift_in' input.

FIFO output -> pixel writer input:
fifo_not_empty -> this should feed the pixel_writer's 'cmd_ready' input.
fifo's data out   -> this should feed the pixel_writer's cmd_input.
the pixel_writer's load_next_cmd output should feed the fifo's 'shift_out'

This fifo should have it's underflow and overflow protection turned on.

I also want 4 new 8 bit input ports added to the Z80 bridge.  You will feed FIFO status flags here and 2 collision counters from the pixel writer unit.
Also make a strobe for each read port so you may use that strobe to see a read and it may be used to clear counters or status bits.

I also want to see a 16 bit output port added to the Z80 but.  Basically 2 adjacent 8 bit ports, with a write strobe output for each.  (This is so you can select loading data into the geo-unit after the low byte is sent, or, after the high byte is sent since the geo unit needs to take in 16bit at a time)  If you do not want to 2 ports to write to the geometry_xy_plotter, but a memory address bytes instead, you will need to do this on your own later.  This 16 bit port will feed a 512x16 word ALT_FIFO megafunction into the geometry_xy_plotter module.

You can test your added Z80 in and out ports by wiring them to the RS232 debugger's real-time in and out ports.