Closed sy2002 closed 4 years ago
Thank you for the detailed description.
As mentioned in Issue #108, the current VGA interrupt mechanism is completely broken (it corrupts the data bus), and I'm in the process of re-implementing it according to the above description. However, I'm running into some problems, so I would like to have some things clarified related to when the ISR address is being presented to the CPU.
In the following I'm referring to this diagram:
Here is my understanding:
INT_N
low when it wants to signal an interrupt.GRANT_N
low and then waits for the ISR address from the device.INT_N
.GRANT_N
, and then starts fetching the next instruction.I notice that when the VGA module is connected as the last device in the daisy chain, the GRANT_N
input to the VGA module is delayed some clock cycles compared to GRANT_N
coming from the CPU, and therefore when the CPU releases GRANT_N
, the VGA module is still pushing the ISR address onto the data bus, which leads to corruption.
So it appears there is a requirement that the GRANT_N
signal be passed combinatorially through the entire daisy chain. This requirement should be made more explicit in the description of the daisy chain process. This requirement is seemingly not obeyed by the TImer device in timer.vhd
. I remember an earlier version of timer.vhd
where the GRANT_N
signal was connected combinatorially but changed to registered in order to break a combinatorial loop. It would seem that the register must be placed on the INT_N
signal rather than the GRANT_N
signal.
I would presume the same problem exists with the Timer 1 module since it too receives a (slightly less) delayed version of GRANT_N
. I have not yet tried to confirm this experimentally, but it could perhaps be as easy as placing a Timer 1 ISR around address 0xE500
like in Issue #108.
And a more general comment about the interrupt daisy chaining architecture:
Currently we have two timers and a VGA module. Soon we will have an Ethernet module too. All these devices are required to adhere to the same strict handshaking protocol whose implementation is non-trivial. So as not to have to spend too much time debugging hardware interrupt implementation, I have attempted to make a general daisy chaining module vhdl/daisy_chain.vhd in a new branch dev-vga-int
.
The idea is that this module is responsible for all the intricacies of the daisy chaining protocol, whereas the individual devices (Timer, VGA, Ethernet, etc.) may remain agnostic about the overall interrupt architecture of the system. You can see an example of how this is connected in the file vhdl/vga_multicolour.vhd.
In other words, future devices (Ethernet, etc) who want to issue interrupts, should instantiate this daisy chaining module, and then everything will work out-of-the-box.
Excellent thoughts!
I particularly love your idea about having a general daisy chain module that everybody can and should use.
So here is my proposed the roadmap about this issue:
[x] You and I discuss all aspects of https://github.com/sy2002/QNICE-FPGA/issues/104#issuecomment-688019689 and https://github.com/sy2002/QNICE-FPGA/issues/104#issuecomment-688023695 and you walk me through your code. Four eyes priciple. Preferably today evening 21:00 in our online meeting
[x] V1.7: Both of us in a screensharing session (also preferably today) update the doc/int-device.md
, so that we have a 100% reliable "law" documentation in develop
.
[x] V1.7: I am updating the timer interrupt modules using your daisy_chain.vhd
in dev-vga-colour
and in the V1.7 version of doc/int-device.md
we should also mention the module daisy_chain.vhd
, describe its usage and only provide the documentation of how to do an own version for future reference. Some sentence like "Implementing the correct daisy chain protocol is not trivial and can lead to non obvious hard to find bugs. This is why we suggest that you use our rock solid and proven to work daisy_chain.vhd
instead of creating your own version."
Fixed in the branch dev-vga-int.
Done with commit e9d4f3deeafca2e4766806e924437044795003bf
I did some extended smoke tests on hardware, here is what works:
✅ The Smoke Test (including loading Q-TRIS via SD/FAT32 while the timer interrupt is running)
✅ fancy stand alone, mode F
and then mode B
✅ fancy in parallel with Q-TRIS (but due to issue #109, Q-TRIS was already in memory)
✅ large file load: qbin/adventure.out from SD Card while test_programs/timer_test
is running and playing it
Here is what does not work, but these are separate issues that have been there before the Daisy chain module was implemented in the timer that do not prevent this one from closing:
Currently, the way VGA is implemented, it ignores the mechanisms of our Daisy-chained architecture. So the TODO of this issue is to implement it. This issue blocks issue #17
Here is a description of how the mechanism / protocol works:
https://github.com/sy2002/QNICE-FPGA/blob/develop/doc/int-device.md#daisy-chaining
I copied the most important parts here, the full info is in
doc/int-device.md
:Daisy chaining
The basic idea of the Daisy chaining protocol used at QNICE-FPGA is that no device is aware of its "position" within the Daisy chain. It might be located right next to the CPU or it might be located "far away".
Every interrupt capable device must support the following signals:
In this description, "left" is a device (or the CPU) that has the authority to grant an interrupt our device. And the "right" device is a device "right" next to our device, which is dependent on our device granting an interrupt to it. Ultimately this authoritry of course stems from the CPU. Due to the very nature of the Daisy chain mechanism, an interrupt request is passed from device to device to "the left" until it reaches the CPU and an interrupt grant is passed from device to device to "the right" until it reaches the requesting device.
There are two very important mechanisms, that need to be built into all interrupt capable devices are "Pass-Through" and "Wait-until-it-is-our-turn":
Pass-Through: If the device is idle regarding interrupts, i.e. it does not need to request an interrupt, then it just passes through the
int_n_in
(interrupt request output from the "right" device that is an input for our device) to theint_n_out
(interrupt request input of the "left" device that is an output for our device). Additionally, thegrant_n_in
(interrupt grant output from the "left" device, that is an input for our device) to thegrant_n_out
(interrupt grant input of the "right" device, that is an output for our device).Wait-until-it-is-our-turn: This mechanism might require a more complex logic, so do not underestimate the subtleties: If our device needs to request an interrupt (e.g. due to a timer that has fired or due to a VGA scanline that has been reached), it might happen that we need to wait rather long until it is our turn. The CPU might be busy with active ISRs or devices with higher priorities (left of us) might be also waiting to issue their request. Or - and this needs special attention - the device that is "right" of us already started its interrupt request during our "idle" phase (as described above). In such a case, we need to monitor the activity that is already taking place and only if the already running activity is over, we can request an interrupt. In other words: Our device must never interfere with an already active transaction of a device that is located to "the right" of us. Depending on your requirements and the behavior you would like to implement, this whole section means, that you might need a FIFO buffer to queue waiting interrupts or other mechanisms to avoid blocking your device while it waits for its turn.
All of this means also: The "closer" a device is to the "left" (i.e. near to the CPU), the higher is the priority of its interrupts.
There is a well-commented reference implementation of the Daisy chain handling in
vhdl/timer.vhd
in the processfsm_output_decode
.