#2203 VIA timer does not wrap to $ffff

[gpz]

Picked this up on Denial...

https://sleepingelephant.com/ipw-web/bulletin/bb/viewtopic.php?p=125437

this game:

https://retrocollector.org/tap/576.zip (+3k)
https://retrocollector.org/tap/577.zip (+8k)

load in xvic, when it says "press fire to start" do so, it counts up some value, then hangs -
apparently the timer high byte in $9125 never becomes $ff

Apparently the game relies on the kernal to initialize the timer/latch with $4826 (this works) and then waits for the highbyte (9125) to become $ff (which never happens)

Strange that the existing tests do not catch this behaviour - this seems to be very basic stuff :)

[Michael Kircher]

The game code uses a very iffy construction to begin with.

The loop at $203A (LDA $9125/CMP #$FF/BNE $203A) takes 4+2+3 = 9 cycles for each test, the value $FFFF - and thus the high byte being $FF - appears in the timer only for a single cycle before it is reloaded from the latch value.

Now the standard value of $4826 (PAL) loaded into the timer latch results in a timer period of $4828 and it's pure luck that period isn't divisible by 9, otherwise that loop could only ever terminate in 1 of 9 instances.

Even so, it will take a variable number of up to 9 actual wraparounds of the timer until high byte = $FF is 'caught'. Why didn't the game coders just use the interrupt facility here?

(Edit: the latch value for NTSC also is 'uncritical' in that regard: here it is $4289, resulting in a period of $428B cycles, again not commensurate to 9)

[Olaf Seibert]

The VIA core code is very old and from the time that every emulator cycle had to be saved where possible. So it doesn't keep track of the value of Timer 1 all the time. It calculates it, when needed, from the difference between current clock cycle, and the next time that T1 is supposed to be reloaded from the latch (this value is called t1reload).

The calculation, in the function viacore_t1() reads as follows:

return via_context->t1reload - rclk - FULL_CYCLE_2;

(there is another calculation for when the clock has already passed t1reload time, but that is not relevant here).

This formula will give -1 (0xFFFF) when rclk is 1 before t1reload. So far so good.

However, there is an alarm set at the time that the timer reaches value 0000 (viacore_t1_zero_alarm()) which pushes t1reload ahead by a full cycle time. (I mean a full set of timer values with that, so essentially the latch value + 2)

So the case where t1reload is 1 ahead of the current time never happens. We are too late. Both t1reload and t1zero have been pushed forward).

Now you could look at the timer's latch value in viacore_t1() to try to see if this moving forward has already happened, and then undo it. But looking at the latch value is dangerous, because it can be written to from the CPU, but it only gets loaded when the timer underflows. So you could find a value there that is not right for the current loop of the timer.

So this needs a bit of thinking.

[Querino]

It seems the game worked up to r41995

[Olaf Seibert]

@querino: interesting. I have my eye on an optimization that can be disabled and which then seems to fix this issue, but that was already present before the change of r41995.

The names have been changed though. In that change, it changed via_context->tau += via_context->tal + 2; to via_context->tau += full_cycle; which should be the same thing.

In current versions, tau has been renamed to t1reload.

So you can try commenting out the line which says via_context->t1reload += full_cycle; which would hopefully break nothing else. I am still pondering that, though.

[Querino]

Sorry i messed up, it's actually r41808.
So there were other changes to viacore, such as in r41932 (many).

/edit:
Ok, tried something more.

viacore:
[r41808] : Game works as expected
[r41932] : Game won't even start/load properly (?)
[r41995] : Same behavior as in current versions, hangs after pressing fire

[gpz]

related tests added in r45976 (testprogs/VIC20/via_wrap/)

[Olaf Seibert]

When I compare versions r41808 with r41995, it looks like I have moved the "timer 1 zero alarm" 1 cycle earlier:

In the case case VIA_T1CH: /* Write timer A high */, it was
via_context->tai = rclk + via_context->tal + 2; and became
via_context->tai = rclk+1 + via_context->tal ;
(tai is now called t1zero)

I could move it 1 later again but this would affect other things that happen in the viacore_t1_zero_alarm() function, such as setting the T1 flag in the Interrupt Flag Register and adjusting the PB7 I/O pin.

Moving the alarm 1 later breaks some viavarious tests: 10, 11, 12, 13, 21. So that isn't a solution.

[gpz]

The PB7 tests never worked though, did they? :=)

[Olaf Seibert]

r45978 contains essentially the change as described above, plus a bunch of changed or added comments that (hopefully) explain why the change is correct.

[Querino]

just a quick test, done manually.

• all in viavarious pass
• the four new wrap test pass
• the game works

waiting for someone doing a more thorough test. :)

[gpz]

r45982 also fixes viavarious 10-14 from the drive tests.

can we close this? :)

[Olaf Seibert]

Closing since it seems to be fixed :)