Undo jt49_cen update.

[greyrogue]

This update made the audio unstable. Might be related to using negative_edge clock? Going back to stable version.

[jotego]

Don’t undo that! The other one is broken because the clock enable signal can be on for more than one cycle. Please check the timing analysis report before assuming that is a problem. It is working correctly on all the CAPCOM cores.

El El vie, 15 nov 2019 a las 5:13, sorgelig notifications@github.com escribió:

Merged #53 https://github.com/MiSTer-devel/NeoGeo_MiSTer/pull/53 into master.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MiSTer-devel/NeoGeo_MiSTer/pull/53?email_source=notifications&email_token=AAOG27CTT65D4GWPJ4QH7DDQTYOWBA5CNFSM4JNVHZW2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOU344H6Q#event-2801386490, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOG27BO2VYF6XNKQDBFXQLQTYOWBANCNFSM4JNVHZWQ .

[sorgelig]

No problem with timings in TQ. It's something wrong in the code.

[sorgelig]

Not sure, but may be worth to use ym2149 module i use in many cores instead. ZX Spectrum is very sensitive to PSG work. I didn't try latest jt49 yet, but it never worked before in ZX Spectrum.

[jotego]

Why do you think that cen generation module is wrong?

El El vie, 15 nov 2019 a las 8:28, sorgelig notifications@github.com escribió:

Not sure, but may be worth to use ym2149 module i use in may cores instead. ZX Spectrum is very sensitive to PSG work. I didn't try latest jt49 yet, but it never worked before in ZX Spectrum.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MiSTer-devel/NeoGeo_MiSTer/pull/53?email_source=notifications&email_token=AAOG27BUDFHGADI6KXQN7D3QTZFQJA5CNFSM4JNVHZW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEESEYQ#issuecomment-554246754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOG27F4FEXXZVCVBLZDKDLQTZFQJANCNFSM4JNVHZWQ .

[greyrogue]

To be clear, I don't see anything wrong with the logic itself. I just thought using negative_edge of the clock was causing instability elsewhere in the core (clock alignment issues, maybe?). It seems that when using the updated version, something goes wrong with communication somewhere (between 68K and Z80 or between Z80 and YM2610) after running for a while. With the old version, this doesn't happen. Or at least that's what a limited amount of testing seemed to show. Also, I do want to get something that works everywhere to get this in line with the original. Using the old version is just a stop-gap solution.

[jotego]

Clock enable signals should change in the negative edge if they gate a positive-edge flip flop. This is done precisely to avoid glitches. If the cen signal changes in the positive edge, then it may create glitches or the compiler may be smart enough to deal with it as a regular LUT input rather than a clock enable signal. So it kind of demotes the signal rank. Clock enable signals are special because they don't go through the LUT and actually stop the flip flop from toggling. For this reason they save power and space. I deliberately try not to use any clock gating the CPU interfaces in order to avoid communication problems.

[sorgelig]

while some docs say it's better to use negedge for CE, it still bad for design. Timings for negedge registered signals are calculated at twice higher clock. Actually compiler is smart enough to add required delays for clock/CE if CE is generated in posedge. I've already started to modify my cores to eliminate negedge. It helps a lot in timing improvements.

Actually in NeoGeo there are no slacks with both posedge and negedge versions. So probably it's not timings, but logic related.

[jotego]

I don’t agree with that change. Clock enable should toggle on the opposite edge of the clock if the cen input of the flip flop is used. My cores will follow that rule.

El El vie, 15 nov 2019 a las 18:41, sorgelig notifications@github.com escribió:

while some docs say it's better to use negedge for CE, it still bad for design. Timings for negedge registered signals are calculated at twice higher clock. Actually compiler is smart enough to add required delays for clock/CE if CE is generated in posedge. I've already started to modify my cores to eliminate negedge. It helps a lot in timing improvements.

Actually in NeoGeo there are no slacks with both posedge and negedge versions. So probably it's not timings, but logic related.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MiSTer-devel/NeoGeo_MiSTer/pull/53?email_source=notifications&email_token=AAOG27HSX7OUADWPOX4TTP3QT3NLJA5CNFSM4JNVHZW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEGFTSQ#issuecomment-554457546, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOG27AAFGDJCU5G5PSVV6TQT3NLJANCNFSM4JNVHZWQ .

[sorgelig]

Negedge is useful only for gated clocks merely for synthesizer to distinguish from async clocks, not for CE. CE is just a single case of wide varieties of inferred CEs. Any if/case is actually CE for specific signals. There are networks for clocks, but there are no networks for CE. So it's the same signal as any other. It has propagation delay, so depending on clock, this delay may diminish negedge after short distance, so fitter has to compensate the delay anyway.

[sorgelig]

@greyrogue are you sure you did only this revert? Code cannot be compiled because parameter CLKDIV is absent in reverted jt49_cen. I can re-add it, no problem.. But it makes me think this revert is not enough. Otherwise how you could compile it?

[greyrogue]

Whoops. Yes you're right, I also commented out the CLKDIV: jt49_cen /#(.CLKDIV(CLKDIV)) / u_cen(

I did the PR by hand, and forgot that minor change.

[sorgelig]

i've released it.. we'll see the feedbacks.