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

[Marco Gerards]

Hi Robert,

Robert Millan <address@hidden> writes:

You forgot the changelog entry :-)

> 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.

If this works and fixes coreboot, it is fine to me ;-)

One comment follows below.

> --- ../grub2.tsc/include/grub/i386/pit.h      1970-01-01 01:00:00.000000000 
> +0100
> +++ ./include/grub/i386/pit.h 2008-08-04 01:22:12.000000000 +0200
> @@ -0,0 +1,19 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +extern void grub_pit_wait (grub_uint16_t tics);

Please us an inclusion guard.

--
Marco