Re: + i386-lapic-timer-calibration.patch added to -mm tree

Re: + i386-lapic-timer-calibration.patch added to -mm tree

[Ingo Molnar]

* akpm@osdl.org <akpm@osdl.org> wrote:

> Subject: i386/apic: Rework local apic timer calibration
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> The local apic timer calibration has two problem cases:
> 
> 1. The calibration is based on readout of the PIT/HPET timer to detect 
> the wrap of the periodic tick. It happens that a box gets stuck in the
> calibration loop due to a PIT with a broken readout function.
> 
> 2. CoreDuo boxen show a sporadic PIT runs too slow defect, which results
> in a wrong lapic calibration. The PIT goes back to normal operation once
> the lapic timer is switched to periodic mode.
> 
> Rework the code to address both problems:
> - Make the calibration interrupt driven. This removes the wait_timer_tick
>   magic hackery from lapic.c and time_hpet.c. The clockevents framework
>   allows easy substitution of the global tick event handler for the
>   calibration. This is more accurate than monitoring jiffies. At this
>   point of the boot process, nothing disturbes the interrupt delivery, so
>   the results are very accurate.
> 
> - Verify the calibration against the PM timer, when available by using the
>   early access function. When the measured calibration period is outside
>   of an one percent window, then the lapic timer calibration is adjusted
>   to the pm timer result.
> 
> - Verify the calibration by running the lapic timer with the calibration
>   handler. Disable lapic timer in case of deviation.
> 
> This also removes the "synchronization" of the local apic timer to the
> global tick. This synchronization never worked, as there is no way to
> synchronize PIT(HPET) and local APIC timer. The synchronization by waiting
> for the tick just alignes the local APIC timer for the first events, but
> later the events drift away due to the different clocks. Removing the
> "sync" is just randomizing the asynchronous behaviour at setup time.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Andrew Morton <akpm@osdl.org>

i agree with this method.

one question:

> +	long tapic = apic_read(APIC_TMCCT);
> +	unsigned long pm = acpi_pm_read_early();

is this function call safe if the box has no pm-timer?

otherwise, i have tested this on a couple of boxes, it looks good. This 
patch should also solve some apic-calibration hangs reported against 
Fedora.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: + i386-lapic-timer-calibration.patch added to -mm tree

[Thomas Gleixner]

On Wed, 2006-11-08 at 13:09 +0100, Ingo Molnar wrote:
> one question:
> 
> > +	long tapic = apic_read(APIC_TMCCT);
> > +	unsigned long pm = acpi_pm_read_early();
> 
> is this function call safe if the box has no pm-timer?

That's in the pm-timer-allow-early-access.patch:

Subject: pmtimer: Allow early access to pm timer

+static inline u32 acpi_pm_read_early(void)
+{
+       if (!pmtmr_ioport)
+               return 0;
+       /* mask the output to 24 bits */
+       return acpi_pm_read_verified();
+}

If pmtmr is not available, the function returns 0, so the resulting
delta is 0 and therefor ignored. Same applies, when PMTIMER is disabled
in the config.

	tglx

Re: + i386-lapic-timer-calibration.patch added to -mm tree

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Wed, 2006-11-08 at 13:09 +0100, Ingo Molnar wrote:
> > one question:
> > 
> > > +	long tapic = apic_read(APIC_TMCCT);
> > > +	unsigned long pm = acpi_pm_read_early();
> > 
> > is this function call safe if the box has no pm-timer?
> 
> That's in the pm-timer-allow-early-access.patch:
> 
> Subject: pmtimer: Allow early access to pm timer
> 
> +static inline u32 acpi_pm_read_early(void)
> +{
> +       if (!pmtmr_ioport)
> +               return 0;
> +       /* mask the output to 24 bits */
> +       return acpi_pm_read_verified();
> +}
> 
> If pmtmr is not available, the function returns 0, so the resulting 
> delta is 0 and therefor ignored. Same applies, when PMTIMER is 
> disabled in the config.

ok, looks good.

	Ingo