search

MiSTer-devel / MSX_MiSTer

MSX for MiSTer
41 stars 22 forks source link

Make all reset uses synchronous #10

Closed MP2E closed 4 years ago

MP2E commented 4 years ago

This silences 1 of the 2 remaining async clock warnings in Quartus!

Also attempts to cherry pick small changes to vdp_vga.vhd from OCM-PLD v3.7.1. If I made a mistake there can revert. Also not sure how useful the 1:1 ratiomode is for MiSTer but I figured I'd bring it over in case.

sorgelig commented 4 years ago

You really need to change your text editor or its settings because it modifies lines with or without the reason, so the real patch gets lost in those CR/LF/TAB/SPACE conversions..

MP2E commented 4 years ago

Ok. I'll see what I can do

sorgelig commented 4 years ago

About async reset: Actually there is nothing wrong to use async reset. It's even more natural than sync reset and it doesn't affect the work if it's properly implemented. Sometimes synchronous reset is not desirable because the clock/CE may be not available in reset time and thus reset will never execute.

What is important to look in async reset to avoid some signals treated as clocks (the one you've targeted this fix): 1) all assignment should be from constant values, not from variables or signals. 2) reset block should not include any conditional blocks. If these conditions are met, then async reset won't produce any false clocks/latches.

P.S.: I've tested your new change in scandoubler - it seems works fine now. Your first port broke it so HDMI displayed a black border in scandoubled resolutions, that's why i've reverted that.

MP2E commented 4 years ago

That's good to know, I'll revert the synchronous reset change and find the signal being treated as a clock and fix it. Bit unfortunate I spent hours doing that, but at least now I know. Glad to hear the scandoubler change is fine

EDIT: also these explanations are helping me understand, thanks!

sorgelig commented 4 years ago

Revert or not - is not a simple question. If you want maintain this core deeply involved into specific fixes and tweaks yourself, then probably you would like to re-factor the code to the way you think is more convenient. So you can decide to use sync or async resets and go either route. But if you want to relay on original code from KdL, then you need to understand that time after time you will need to port the changes. If you will change the code a lot without a big reason, then you will have to remember all your changes and mix them with new code. With such changes in many lines it will be not so easy to port more changes from original code.

sorgelig commented 4 years ago

I think R800_MULU generic is redundant and can be safely removed. R800_mode signal already does the same.