V5.1 RC1 vs. Alpha 3: Attack of the PETSCII Robots REU version fails

[sy2002]

• User "Senfsosse" on Discord reported, that the REU version of Attack of the PETSCII Robots is not working any more using RC1 but it did work using Alpha 3.

• Interestingly enough, it works like a charme on my good old R3

• I assume this is one of these nasty "HyperRAM phase shift" bugs due to different HyperRAM revisions used in different R3A batches

• A quick look into doc/inofficial.md shows that in Alpha 4 we changed some HyperRAM stuff, which might be the reason that on some machines the HyperRAM and therefore the REU is not as stable as before.

@MJoergen Assigning this to you as you were the one who changed the HyperRAM stuff in Alpha 4 :-) As far as I remember, you wanted to find a setting that works for all boards (R3/R3A, R4, R5, R6) but I suspect that you now broke compatibility with some revisions of HyperRAM used on R3/R3A boards: For your convenience: Alpha 3 is commit c099921 and Alpha 4 is fdd6ec0.

Maybe we go to one PhaseShift setting per board?

Or any other ideas such as:

@amb5l and @MJoergen should we take this incident to fix HyperRAM stability once and for all?

[sy2002]

@Schwefelholz FYI

[amb5l]

@sy2002 @MJoergen I will compare the Alpha 3 commit with RC1. Can you tell me anything about the different HyperRAM chips that were used on R3/A? Part numbers?

[MJoergen]

The original R3 boards use "IS66WVH8M8BLL-100B1LI". During production of the R3 boards, Trenz changed the part number from "revision B" to a "revision D". See this GitHub issue : https://github.com/MJoergen/HyperRAM/issues/2

[amb5l]

I've looked at Alpha 3 vs the latest RC1/develop. common.xdc includes the HyperRAM timing constraints we worked on together previously.

We are missing a constraint: the FPGA-to-HyperRAM section should constrain hr_rwds_io in the same was as *hr_d_io[]**.

To correct this, add hr_rwds_io to the constraints as follows:

################################################################################
# FPGA to HyperRAM (address and write data)

# setup
set_output_delay -max  $HR_tIS -clock hr_ck [get_ports hr_rwds_io hr_d_io[*]]
set_output_delay -max  $HR_tIS -clock hr_ck [get_ports hr_rwds_io hr_d_io[*]] -clock_fall -add_delay

# hold
set_output_delay -min -$HR_tIH -clock hr_ck [get_ports hr_rwds_io hr_d_io[*]]
set_output_delay -min -$HR_tIH -clock hr_ck [get_ports hr_rwds_io hr_d_io[*]] -clock_fall -add_delay

################################################################################

If this does not fix the issue, could one of you run a timing simulation and inspect the waveforms to see how the implemented design is behaving with respect to the constraints?

[sy2002]

@amb5l Thank you for your analysis! We will create a new Alpha version for @Schwefelholz and let him test if this fixes the issue.

[MJoergen]

When building bitstream with this extra constraint I get a single (large) timing violation of over 700 ps.

I will probably have to refactor some of the code, to move registers into the I/O buffer.

[MJoergen]

Interestingly, the corresponding path on the DQ signal has a large margin, with a positive slack of 8 ns.

[MJoergen]

@amb5l What are your thoughts here? We previously added a set_multicycle_path to the DQ lines and asserted the corresponding Output Enable pin one cycle earlier.

We are already asserting the OE line for the RWDS signal one cycle earlier, just like for the DQ lines. Should we just add a corresponding set_multicycle_path as well ?

[amb5l]

Should we just add a corresponding set_multicycle_path as well ?

Yes, you are right, it should be applied to rwds also. -to [get_ports hr_d_io[*]] should become -to [get_ports hr_rwds_io hr_d_io[*]]

(These set_multicycle_path constraints should be moved into the FPGA-to-HyperRAM section.)

[sy2002]

Infos from Senfsosse on Discord see screenshots

[sy2002]

@Schwefelholz here is an "Alpha 10" test build for R3/R3A machines. Please test if it fixes the issue: C64MEGA65-V5.1A10.zip

[amb5l]

@MJoergen @sy2002 Just to clarify: the VGA output does NOT use HyperRAM, so this issue is somewhere else?

Regardless, it's good that we looked at HyperRAM constraints again, they were not perfect.

[sy2002]

@amb5l Yes, the VGA output does not use HyperRAM. But the C64 RAM Extension Unit aka REU does use HyperRAM, so @MJoergen and I assume that we have a HyperRAM problem here as we are talking about the "REU" version of the game. List of "evidence":

• It worked for Senfsosse / @Schwefelholz with Alpha 3 (which had the same R3/R3A HyperRAM read/write phases than the currently stable release Version 5.0 of the core) and it stopped working for him after we changed the HyperRAM phase to accomodate R4/R5/R6 boards in our currently work-in-progress version 5.1 of the core
• But even the newest work-in-proress 5.1 of the core works perfectly fine for me (I have one of these super reliable "old" HyperRAM chips as I have an old R3, not one of the newer R3A boards)

[MJoergen]

@amb5l We've adopted the change you suggested above. I hope that fixes the issue. If not, my next steps are:

• Perform the post-implementation simulation you requested and inspect the waveforms.
• Optionally refactor the code, starting with lines 106-119 in hyperram_io.vhd. I would like to have registers placed directly in the output pads, so I can use something like the following constraint. I believe that will reduce routing delay and make timing closure easier:

set_property IOB TRUE [get_ports hr_rwds_oe_n_o ]

However, currently the code has logic after the output register, hence the need for a refactoring, which will also involve the file hyperram_ctrl.vhd.

[Schwefelholz]

@sy2002 Seems I cannot get the "Alpha 10" core started at all. Flashed it twice, but the outcome is always the same. Just the background color differs from start to start.

[MJoergen]

@Schwefelholz Does the On Screen Menu work, when the screen is blue like above?

And if this is HDMI output, could you try the VGA output as well, if the result is the same ?

[Schwefelholz]

@MJoergen Idiot me... Yes, the screen menu is working and yes, I get a working core over VGA.

[sy2002]

@Schwefelholz Does "working core over VGA" mean that Attack of the PETSCII Robots works with A10 (without HDMI)?

[Schwefelholz]

I was just referring to the core itself. Still need to test PETSCII Robots. Too little time yesterday. Will give an update later.

[Schwefelholz]

@sy2002 Just checked PETSCII Robots on the A10 core. It doesn't work at all. Using the core with VGA, attaching the 1st disk. LOAD"*",8 is working, but RUNing the program makes the core hang.

It actually should show a boot screen and commence loading from disk. For me, A10 is definitely behaving worse than the RC1 core.

[MJoergen]

@Schwefelholz here is an "Alpha 11" test build for R3/R3A machines. Please test if it fixes the issue:

C64MEGA65-V5.1A11.zip

[Schwefelholz]

I'll be back at the Mega at the earliest tomorrow evening. Will let you know.

[sy2002]

@MJoergen Just as an FYI: I tested Alpha 11 with the game and it works like a charme. But this is not yet great news, because on my machine also RC1 worked like a charme :-) Let's see what @Schwefelholz comes back with.

[Schwefelholz]

@MJoergen @sy2002 The A11 core seems to be working fine for me. I started it and PETSCII Robots five times in a row with no issues on HDMI.

However, the graphics glitch described in issue #86 is still present.

[MJoergen]

@Schwefelholz here is an "Alpha 12" test build for R3/R3A machines. Please test if it fixes the issue:

C64MEGA65-V5.1A12.zip

[Schwefelholz]

@MJoergen "Alpha 12" is a step back to where we already were with "Alpha 10": No output on HDMI at all. And on VGA PETSCII Robots behaves as described in https://github.com/MJoergen/C64MEGA65/issues/127#issuecomment-1919330671

[sy2002]

Since @Schwefelholz confirmed that the refactored HyperRAM from Alpha 11 fixes the issue and that the changed constraints of Alpha 12 are a step back, we are now sticking to the code from Alpha 11 and can close this issue.

P.S. The graphical glitches of issue #86 were also present in Version 5, so they are no regression of 5.1 and therefore not a blocker for the 5.1 release.