Fix 8254 timer inaccuracy

[jwt27]

Not sure who Steve is, but he messed up a bit here.

The problem in #99 is that tv_sec is set from libc time(), while tv_usec is set from microsec_clock(). Those two are of course not guaranteed to be synchronized.

Simply putting #define STEVES_PATCHES 0 is a solution, but removing it altogether makes the code more readable again.

The benchmark (from the other thread) now becomes:

hires_timer(0):  3387089559 cycles (avg 45161ns per call)
hires_timer(1):   234681742 cycles (avg  3129ns per call)
USE_RDTSC=1   :    27900545 cycles (avg   372ns per call)

[jwt27]

Test program:

#include <stdio.h>
#include <tcp.h>

int main()
{
  unsigned long now, last, print;

  init_misc();
  hires_timer(1);

  print = last = set_timeout(0);
  while (!kbhit())
  {
    now = set_timeout(0);

    if (now > last + 1)
    {
      printf("JUMP forward: %lu ms\n", now - last);
      goto print_it;
    }
    else if (now < last)
    {
      printf("JUMP backward: %li ms\n", (long)(now - last));
      goto print_it;
    }

    if (now >= print + 100)
    {
    print_it:
      printf("%lu\n", now);
      print = now;
    }

    last = now;
  }

  return 0;
}

At midnight:

945232350
945232450
945232550
945232650
JUMP backward: -514174617 ms
431058081
JUMP forward: 427725352 ms
858783488
858783588
858783688
858783788

The brief very-negative jump is fixed now:

945182966
945183066
945183166
945183266
JUMP backward: -86399866 ms
858783488
858783588
858783688
858783788

But yes, it does jump 24 hours back.

[jwt27]

More-or-less fixed now:

948783030
948783130
948783230
948783330
JUMP forward: 134 ms
948783488
948783588
948783688
948783788

The remaining jump at midnight is due to the BIOS having an incorrect notion of "24 hours". The exact rollover point should be 1573042.436 ticks, but the BIOS in my machine wraps at 1573040. This leaves 2.436 × 54.92 = 133.81ms. I don't see any way to correct for that.

I think the timezone stuff is also correct now (not that it matters, but hey). Although djgpp's time() seems to add DST which shouldn't apply now.

[gvanem]

Great work! I'll merge this tomorrow.

[gvanem]

Merged and many thanks once again.

[jwt27]

...and thanks again for merging!

Now to quote myself again...

The remaining jump at midnight is due to the BIOS having an incorrect notion of "24 hours". The exact rollover point should be 1573042.436 ticks, but the BIOS in my machine wraps at 1573040. This leaves 2.436 × 54.92 = 133.81ms. I don't see any way to correct for that.

I looked into this some more, that 1573040 number is what IBM originally defined as "24 hours", and so most BIOS implementations follow it.

So you could correct for it, just derive the frequency from that number. Then the best ratio is:

diff --git a/src/gettod.c b/src/gettod.c
index ce0ae0e..aa707be 100644
--- a/src/gettod.c
+++ b/src/gettod.c
@@ -322,7 +322,11 @@ static __inline uint64 microsec_clock (void)
   while (hi != PEEKL(0, BIOS_CLK));   /* tick count changed, try again */
   lo = 0 - lo;
   rc = ((uint64)hi << 16) + lo;
-  return (rc * U64_SUFFIX(88000000) / U64_SUFFIX(105000000));
+
+  /* This magic ratio is chosen to synchronize with what the
+   * BIOS considers "24 hours" (1573040 ticks).
+   */
+  return (rc * 117187500UL / 211416576UL);
 }

 #if (DOSX)

Tiny change, but you can add that if you want.

On the other hand this post claims that 1573041 is also used. So those would jump 55ms backwards.

And I see that 1573067 is used in ms_clock() (timer.c) - that doesn't seem right. It's not mentioned in the reference you added, so I wonder where that came from?

And then in chk_timeout():

      if (hour < oldHour)               /* midnight passed */
      {
#define CORR 290UL                      /* experimental correction */
        date    += 1572750UL + CORR;    /* Update date (0x1800B0UL) */
        date_ms += 3600UL*24UL*1000UL;  /* ~2^26 */
      }

1572750 + 290 = 1573040 = 0x1800B0 so that checks out. Seems there's a lot of code all trying to do the same thing in a different way :)

[gvanem]

And I see that 1573067 is used in ms_clock()

That was just rewrite of the code contributed by "Alain" <alainm@pobox.com>. And ms_clock() is only used in delay() for the High-C and Digital Mars compilers (both pretty dead).

So we should redefine BIOS_TICKS_DAY to 1573040.