Re: TSC on coreboot (Re: [PATCH] High resolution time/TSC patch v3)

[Colin D Bennett <colin@gibibit.com>]

[-- Attachment #1: Type: text/plain, Size: 1815 bytes --]

On Mon, 4 Aug 2008 01:53:48 +0200
Robert Millan <rmh@aybabtu.com> wrote:

> On Sun, Aug 03, 2008 at 09:48:16PM +0200, Robert Millan wrote:
> > On Mon, Jul 28, 2008 at 10:05:33AM -0700, Colin D Bennett wrote:
> > > +/* Calibrate the TSC based on the RTC.  */
> > > +static void
> > > +calibrate_tsc (void)
> > > +{
> > > +  /* First calbrate the TSC rate (relative, not absolute time).
> > > */
> > > +  grub_uint64_t start_tsc;
> > > +  grub_uint64_t end_tsc;
> > > +  grub_uint32_t initial_tick;
> > > +  grub_uint32_t start_tick;
> > > +  grub_uint32_t end_tick;
> > > +
> > > +  /* Wait for the start of the next tick;
> > > +     we'll base out timing off this edge. */
> > > +  initial_tick = grub_get_rtc ();
> > 
> > Ah, I see the problem.  It calls grub_get_rtc() which in
> > grub-coreboot is just a stub.
> > 
> > How about using the interval timer for calibration instead?
> 
> Here.  With this patch your code works on coreboot too.
> 
> Note:  AFAICT we can't calculate the epoch without RTC.  But then
> again, this epoch is just as defined by the time BIOS enables RTC
> interrupts, so why not define it ourselves?  I propose that we define
> epoch as the time in which our TSC code is initialized.
> 
> If knowing the time in which BIOS was started is really useful, maybe
> we could #ifdef it instead.  Though if it's not I'd prefer the
> simplicity.

So far we never use the value returned by ``grub_get_time_ms()`` as an
absolute time.  It is currently only used in relative terms, so only
the rate at which it changes is important, not the definition of the
zero point.  Perhaps that simplifies things?  I think it's fine to go
with that definition for now -- if we need to use absolute time at some
point, we can deal with it then.

Regards,
Colin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]