MEGA65 / mega65-core

MEGA65 FPGA core
Other
240 stars 85 forks source link

VIC-IV raster irq incorrectly triggered twice #604

Closed ki-bo closed 2 years ago

ki-bo commented 2 years ago

Test Environment (required)

Describe the bug

  1. If the IRQ is acknowledged ($d019) while still in the same line where it was triggered and the handler is exited while still in the same raster line, a second "phantom" raster IRQ will be triggered at the beginning of the following raster line. This makes it rather impossible to have proper raster IRQ handlers that last shorter than one raster line.
  2. If the IRQ is acknowledged in the same line where it was triggered, but exited in another raster line, a second (phantom) IRQ will be triggered immediately after the first handler has exited via rti (it is therefore happening in the same raster line still, not the following one).
  3. If the IRQ is acknowledged in a different raster line than it was triggered, all is as expected (no second phantom IRQ is triggered).

To Reproduce Steps to reproduce the behavior: See example code snippet

vic-iv_raster_irq_ack_bug.txt

The raster irq should draw the border in red for some lines starting at $68. Instead, it also draws it cyan for the same amount of lines afterwards - indicating a second raster irq has been triggered after the first one is done. The IRQ handler of the test snippet attached is written to chose red color in case we really get called in line $68 and will draw cyan lines otherwise.

Expected behavior The raster IRQ acknowledge should be possible in the IRQ handler no matter in which raster line it is done (same behaviour as a C64/VIC-II would show). In the attached example: only one raster IRQ should be triggered (in the example in line $68), no other raster IRQ should be triggered, no cyan lines should be visible.

Screenshots Output of the test program: IMG_9849

ki-bo commented 2 years ago

Looking at viciv.vhdl, the problem seems to be that for a vic-iv raster line, the irq is activated for the first three pixels (as long as external_frame_x_zero_latched is 1), but in addition at the very end of the raster. There are two pixelclock cycles where external_frame_x_zero_latched is already 1, but ycounter is still representing the old raster line (since ycounter is updated through ycounter_driver, there is a two cycle delay). This will trigger the raster irq again at the very beginning of the next raster, and therefore only irq handlers lasting at least until the second row will be triggered as expected by users (and only if users are careful to ack the irq only in the next line, thus rather at the end of the irq handler).

This is not an issue for vic-ii raster lines, because for those the raster irq is edge triggered on the update of ycounter (or more specifically vicii_ycounter in that case), so there is only one specific pixelclock cycle in which the raster irq triggering is evaluated. There is a comment in the code that for vic-iv this is not needed. But I really can't see why this should not also be the case for vic-iv rasters. We will eliminate the spurious raster double triggers and make life easier for developers getting the behaviour they are used to.