Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sample rdaddr_r at the rising edge of bus_edge #8

Closed
wants to merge 1 commit into from

Conversation

jburks
Copy link
Contributor

@jburks jburks commented Apr 3, 2022

Fix for #4 and possibly #5

This change moves sampling of the external address bus from the negative edge of bus_read to the positive edge. Implicit with this change is that the address bus is stable when CS# and/or RD# are asserted.

This change was validated against the emulator and all reads matched between the hardware and the emulator. This testing environment is somewhat slow due to the slow path setting the bits of the simulated bus over I2C which makes stress-testing this fix difficult. It should be validated on real hardware.

@fvdhoef
Copy link
Owner

fvdhoef commented Apr 27, 2022

I don't have any hardware set up to validate this. On the real X16 the decoding logic on the motherboard is quite deep and adds a lot of uncertainty to the timing. So I can't just merge this without testing on real hardware.

@fvdhoef
Copy link
Owner

fvdhoef commented Sep 20, 2022

Is this pull request still needed? Is it tested on the 65c02?

@jburks
Copy link
Contributor Author

jburks commented Sep 20, 2022

This has been tested on both 65C02 and 65C816 on my hardware. What most needs to be verified is that this works on the official proto3 hardware. I believe the last few VERA bitstream files I sent Kevin had this change and it worked on his proto3 board. There's a chance that I remember incorrectly though. I received what should be the last critical components needed to build out the proto3 board I have today and yesterday I assembled one of my boards to send to David for testing.

I would like to wait a few more weeks on this one so that I can build a proto3 board and verify that it works -- just in case I'm mistaken about the bitstream I sent Kevin. I suspect that my recommendation will be to accept this PR.

Speaking more theoretically: it seems that any sensible design will synchronize the read enable signal with PHI2. It's what my design does; what Kevin's design does; and what is recommended in various corners of the internet. However, this comes with the same problem addressed in the recent 65C816 timing: the rising edge of /RE will arrive very close to the end of the 65C02 address hold time.

For data bus signals during writes on a 65C02, this isn't as big of an issue because while the datasheet states that the CPU can start changing data bus 10ns after the falling edge of PHI2, in reality this only ever happens when return addresses are being pushed onto the stack. This isn't true of the address bus.

Logic analyzer captures of a 65C02 show that some address signals will start to change as early as 12ns +/- 2ns past the falling edge of PHI2 (my logic analyzer samples at 500MHz so my precision is limited to 2ns). Sampling the address bus on the positive edge of bus_read means sampling very close to the end of the address hold time. If the board's address wires are short, or otherwise have low capacitance and can change quickly, there is a good chance that the address bus will get sampled while the address bus signals are changing.

The 65C02 and 65C816 both guarantee that the address bus is stable prior to the rising edge of PHI2. (At 8MHz, the address bus should be stable 32.5ns prior to the rising edge of PHI2.) Since /RE is synchronized to PHI2, we can infer that /RE will assert a few ns after the rising edge of PHI2, and thus should always sample a bus that has been stable for 32.5ns and that will continue to be stable for approximately 72.5ns past the rising edge of PHI2. This PR should be highly reliable and not subject to tight timing conditions as is the case with sampling the data bus during writes.

All that theory said - it should still be confirmed on the proto3 hardware in case I got something wrong here.

@jburks
Copy link
Contributor Author

jburks commented Nov 13, 2022

I don't want to leave this PR open for an unnecessarily long time. I recently went through the pre-synthesis constraints and, after making the changes that I believe specify the relationship between /RE, /WE, and the data bus, it appears that Radiant is generating a design that passes on both the official X16 board and my X16-compatible board. While the timing for those is still very tight, I no longer have a clear need for this change.

Any objection to closing this PR without merging?

@fvdhoef fvdhoef closed this Nov 14, 2022
jburks pushed a commit to jburks/vera-module that referenced this pull request Jul 24, 2023
akumanatt added a commit to akumanatt/vera-module that referenced this pull request Nov 4, 2023
* refactored video modulator to use 1 less DSP and lessening the timing pressure for inputs of y_s, i_s and q_s flip flops

* added module for a pair of 8x8 unsigned multipliers

* rename of wires to make it more clear and explicit

* small cleanup

* small cleanup

* changed version number to v0.1.2

* Gave negated signals a suffix to indicate they contain the negated value of their intended value

* LUT usage reduction by refactoring and reordering

* moved address and data related logic to separate addr_data module

* LUT usage reduction by refactoring and reordering (fvdhoef#5)

* Revert: Increase audio mixing resolution (fvdhoef#7)

Co-authored-by: Natt Akuma <[email protected]>

* FX: Separate address and data logic into module (fvdhoef#8)

* LUT usage reduction by refactoring and reordering

* moved address and data related logic to separate addr_data module

* replaced another two multiplications with adders

* replaced address logic structure with extendable one, added (dummy) addr1-mode

* FX extendable structure for address logic (fvdhoef#9)

* LUT usage reduction by refactoring and reordering

* moved address and data related logic to separate addr_data module

* replaced another two multiplications with adders

* replaced address logic structure with extendable one, added (dummy) addr1-mode

* added 8bpp line draw mode

* replaced tabs by spaces

* changed naming and added comments

* [FX] Line draw mode (8bpp) (fvdhoef#10)

* LUT usage reduction by refactoring and reordering

* moved address and data related logic to separate addr_data module

* replaced another two multiplications with adders

* replaced address logic structure with extendable one, added (dummy) addr1-mode

* added 8bpp line draw mode

* replaced tabs by spaces

* changed naming and added comments

* added polygon filler helper (8bpp)

* typo and more explicit check if ADDR0/1 is untouched

* [FX] Polygon filler mode (8bpp) (fvdhoef#12)

* LUT usage reduction by refactoring and reordering

* moved address and data related logic to separate addr_data module

* replaced another two multiplications with adders

* replaced address logic structure with extendable one, added (dummy) addr1-mode

* added 8bpp line draw mode

* replaced tabs by spaces

* changed naming and added comments

* added polygon filler helper (8bpp)

* typo and more explicit check if ADDR0/1 is untouched

* added affine mode (8bpp)

* added 32-bit cache and multiplier/accumulator, including transparent writes and 16bit hop mode

* added 4-bit and 2-bit modes

* Bump version (fvdhoef#17)

* Bump version

* Set version to 0.3.1

---------

Co-authored-by: Jeffrey <[email protected]>
Co-authored-by: Joe Burks <[email protected]>
Co-authored-by: Natt Akuma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants