MiSTer-devel / SMS_MiSTer

Sega Master System for MiSTer
45 stars 43 forks source link

VDP is runing 3,33% faster than it should #168

Closed ducasp closed 9 months ago

ducasp commented 9 months ago

This code is not correct:

always @(negedge clk_sys) begin reg [4:0] clkd;

ce_sp <= clkd[0];
ce_vdp <= 0;//div5
ce_pix <= 0;//div10
ce_cpu <= 0;//div15
clkd <= clkd + 1'd1;
if (clkd==29) begin
    clkd <= 0;
    ce_vdp <= 1;
    ce_pix <= 1;
end else if (clkd==24) begin
    ce_cpu <= 1;  //-- changed cpu phase to please VDPTEST HCounter test;
    ce_vdp <= 1;
end else if (clkd==19) begin
    ce_vdp <= 1;
    ce_pix <= 1;
end else if (clkd==14) begin
    ce_vdp <= 1;
end else if (clkd==9) begin
    ce_cpu <= 1;
    ce_vdp <= 1;
    ce_pix <= 1;
end else if (clkd==4) begin
    ce_vdp <= 1;
end

end

It will divide 53.6whateverMHz into 29 slices instead of 30, because on 29 it turns value to 0, and then the first thing on the next clock is that 0 being increased to 1, so the "0" condition is never evaluated and then it goes from 1 to 29 (so 29 slices), and so on.

There are several ways to fix this, where I'm using this code I've chosen to change it to this, which fixes and make video run on the correct speed all the time:

always @(negedge clk_sys) begin reg [4:0] clkd;

ce_sp <= clkd[0];
ce_vdp <= 0;//div5
ce_pix <= 0;//div10
ce_cpu <= 0;//div15
clkd <= clkd + 1'd1;
if (clkd==29) begin
    clkd <= 5'b11111;
    ce_vdp <= 1;
    ce_pix <= 1;
end else if (clkd==24) begin
    ce_cpu <= 1;  //-- changed cpu phase to please VDPTEST HCounter test;
    ce_vdp <= 1;
end else if (clkd==19) begin
    ce_vdp <= 1;
    ce_pix <= 1;
end else if (clkd==14) begin
    ce_vdp <= 1;
end else if (clkd==9) begin
    ce_cpu <= 1;
    ce_vdp <= 1;
    ce_pix <= 1;
end else if (clkd==4) begin
    ce_vdp <= 1;
end

end

So when 29 is reached, next clock arrives, 11111 rolls over to 00000 and you have 30 slices, so the clock enables are correctly as VDP is activated 6 times in 30 slices (so 1/5) and Pixel clock is activated 3 times in 30 slices (so 1/10)

Hope that helps your project!

ducasp commented 9 months ago

Never mind, this is not an issue when this is used in VERILOG or SYSTEM VERILOG, just if you change that logic for VHDL and use a variable in place of a register, my bad, no issue to be found here, sorry for the false alarm