On Wed, 25 Aug 2021, Mark Cave-Ayland wrote:
> On 24/08/2021 11:09, Finn Thain wrote:
>
> > The first reload of timer 1 is early by half of a clock cycle as it gets
> > measured from a falling edge. By contrast, the succeeding reloads are
> > measured from rising edge to rising edge.
> >
> > Neglecting that complication, the behaviour of the counter should be the
> > same from one reload to the next. The sequence is always:
> >
> > N, N-1, N-2, ... 2, 1, 0, -1, N, N-1, N-2, ...
> >
> > But at the first reload, the present driver does this instead:
> >
> > N, N-1, N-2, ... 2, 1, 0, -1, N-1, N-2, ...
> >
> > Fix this deviation for both timer 1 and timer 2, and allow for the fact
> > that on a real 6522 the timer 2 counter is not reloaded when it wraps.
> >
> > Signed-off-by: Finn Thain <fth...@linux-m68k.org>
> > ---
> > hw/misc/mos6522.c | 19 +++++++++++--------
> > 1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> > index 5b1657ac0d..0a241fe9f8 100644
> > --- a/hw/misc/mos6522.c
> > +++ b/hw/misc/mos6522.c
> > @@ -63,15 +63,16 @@ static unsigned int get_counter(MOS6522State *s,
> > MOS6522Timer *ti)
> > if (ti->index == 0) {
> > /* the timer goes down from latch to -1 (period of latch + 2) */
> > if (d <= (ti->counter_value + 1)) {
> > - counter = (ti->counter_value - d) & 0xffff;
> > + counter = ti->counter_value - d;
> > } else {
> > - counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
> > - counter = (ti->latch - counter) & 0xffff;
> > + int64_t d_post_reload = d - (ti->counter_value + 2);
> > + /* XXX this calculation assumes that ti->latch has not changed
> > */
> > + counter = ti->latch - (d_post_reload % (ti->latch + 2));
> > }
> > } else {
> > - counter = (ti->counter_value - d) & 0xffff;
> > + counter = ti->counter_value - d;
> > }
> > - return counter;
> > + return counter & 0xffff;
> > }
> > static void set_counter(MOS6522State *s, MOS6522Timer *ti, unsigned int
> > val)
> > @@ -103,11 +104,13 @@ static int64_t get_next_irq_time(MOS6522State *s,
> > MOS6522Timer *ti,
> > /* the timer goes down from latch to -1 (period of latch + 2) */
> > if (d <= (ti->counter_value + 1)) {
> > - counter = (ti->counter_value - d) & 0xffff;
> > + counter = ti->counter_value - d;
> > } else {
> > - counter = (d - (ti->counter_value + 1)) % (ti->latch + 2);
> > - counter = (ti->latch - counter) & 0xffff;
> > + int64_t d_post_reload = d - (ti->counter_value + 2);
> > + /* XXX this calculation assumes that ti->latch has not changed */
> > + counter = ti->latch - (d_post_reload % (ti->latch + 2));
> > }
> > + counter &= 0xffff;
> > /* Note: we consider the irq is raised on 0 */
> > if (counter == 0xffff) {
>
> I think the code looks right, but I couldn't see an explicit reference to this
> behaviour in
> http://archive.6502.org/datasheets/mos_6522_preliminary_nov_1977.pdf.
Yes, that datasheet is missing a lot of information.
> Presumably this matches what you've observed on real hardware?
>
Yes.
