From 399e1586bec7bb6f57c34521fc21295c91c6965a Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Wed, 23 Aug 2017 19:16:27 +0200 Subject: [PATCH] timer: generic timer_ticks_to_us implementation There are hardware timers whose frequency can't be expressed as ticks-per-microsecond integer-value because only a ticks-per-millisecond integer-value is precise enough. We don't want to use expensive floating-point values here but nonetheless want to translate from ticks to time with microseconds precision. Thus, we split the input in two and translate both parts separately. This way, we can raise precision by shifting the values to their optimal bit position. Afterwards, the results are shifted back and merged together again. As this algorithm is not so trivial anymore and used by at least three timer drivers (base-hw/x86_64, base-hw/cortex_a9, timer/pit), move it to a generic header to avoid redundancy. Ref #2400 --- .../base-hw/src/core/spec/cortex_a9/timer.cc | 31 +++-------- repos/base-hw/src/core/spec/x86_64/timer.cc | 27 ++-------- repos/base/include/drivers/timer/util.h | 53 +++++++++++++++++++ repos/os/src/drivers/timer/pit/time_source.cc | 26 ++------- 4 files changed, 67 insertions(+), 70 deletions(-) create mode 100644 repos/base/include/drivers/timer/util.h diff --git a/repos/base-hw/src/core/spec/cortex_a9/timer.cc b/repos/base-hw/src/core/spec/cortex_a9/timer.cc index fb4a9d167..8cae627e7 100644 --- a/repos/base-hw/src/core/spec/cortex_a9/timer.cc +++ b/repos/base-hw/src/core/spec/cortex_a9/timer.cc @@ -12,6 +12,9 @@ * under the terms of the GNU Affero General Public License version 3. */ +/* Genode includes */ +#include + /* core includes */ #include #include @@ -23,6 +26,7 @@ using namespace Kernel; Timer_driver::Timer_driver(unsigned) : Mmio(Platform::mmio_to_virt(Board::Cpu_mmio::PRIVATE_TIMER_MMIO_BASE)) { + static_assert(TICS_PER_MS >= 1000, "Bad TICS_PER_MS value"); write(0); } @@ -44,31 +48,8 @@ void Timer::_start_one_shot(time_t const ticks) } -time_t Timer::_ticks_to_us(time_t const ticks) const -{ - /* - * If we would do the translation with one division and - * multiplication over the whole argument, we would loose - * microseconds granularity although the timer frequency would - * allow for such granularity. Thus, we treat the most significant - * half and the least significant half of the argument separate. - * Each half is shifted to the best bit position for the - * translation, then translated, and then shifted back. - */ - static_assert(Driver::TICS_PER_MS >= 1000, "Bad TICS_PER_MS value"); - enum { - HALF_WIDTH = (sizeof(time_t) << 2), - MSB_MASK = ~0UL << HALF_WIDTH, - LSB_MASK = ~0UL >> HALF_WIDTH, - MSB_RSHIFT = 10, - LSB_LSHIFT = HALF_WIDTH - MSB_RSHIFT, - }; - time_t const msb = ((((ticks & MSB_MASK) >> MSB_RSHIFT) - * 1000) / Driver::TICS_PER_MS) << MSB_RSHIFT; - time_t const lsb = ((((ticks & LSB_MASK) << LSB_LSHIFT) - * 1000) / Driver::TICS_PER_MS) >> LSB_LSHIFT; - return msb + lsb; -} +time_t Timer::_ticks_to_us(time_t const ticks) const { + return timer_ticks_to_us(ticks, Driver::TICS_PER_MS); } unsigned Timer::interrupt_id() const { diff --git a/repos/base-hw/src/core/spec/x86_64/timer.cc b/repos/base-hw/src/core/spec/x86_64/timer.cc index 9d3375d76..95b9e98b4 100644 --- a/repos/base-hw/src/core/spec/x86_64/timer.cc +++ b/repos/base-hw/src/core/spec/x86_64/timer.cc @@ -15,6 +15,9 @@ #include +/* Genode includes */ +#include + /* core includes */ #include #include @@ -85,29 +88,7 @@ void Timer::_start_one_shot(time_t const ticks) { time_t Timer::_ticks_to_us(time_t const ticks) const { - - /* - * If we would do the translation with one division and - * multiplication over the whole argument, we would loose - * microseconds granularity although the timer frequency would - * allow for such granularity. Thus, we treat the most significant - * half and the least significant half of the argument separate. - * Each half is shifted to the best bit position for the - * translation, then translated, and then shifted back. - */ - enum { - HALF_WIDTH = (sizeof(time_t) << 2), - MSB_MASK = ~0UL << HALF_WIDTH, - LSB_MASK = ~0UL >> HALF_WIDTH, - MSB_RSHIFT = 10, - LSB_LSHIFT = HALF_WIDTH - MSB_RSHIFT, - }; - time_t const msb = ((((ticks & MSB_MASK) >> MSB_RSHIFT) - * 1000) / _driver.ticks_per_ms) << MSB_RSHIFT; - time_t const lsb = ((((ticks & LSB_MASK) << LSB_LSHIFT) - * 1000) / _driver.ticks_per_ms) >> LSB_LSHIFT; - return msb + lsb; -} + return timer_ticks_to_us(ticks, _driver.ticks_per_ms); } time_t Timer::us_to_ticks(time_t const us) const { diff --git a/repos/base/include/drivers/timer/util.h b/repos/base/include/drivers/timer/util.h new file mode 100644 index 000000000..b20b620ca --- /dev/null +++ b/repos/base/include/drivers/timer/util.h @@ -0,0 +1,53 @@ +/* + * \brief Utilities for timer drivers + * \author Martin Stein + * \date 2017-08-23 + */ + +/* + * Copyright (C) 2017 Genode Labs GmbH + * + * This file is part of the Genode OS framework, which is distributed + * under the terms of the GNU Affero General Public License version 3. + */ + +#ifndef _DRIVERS__TIMER__UTIL_H_ +#define _DRIVERS__TIMER__UTIL_H_ + +namespace Genode { + + /* + * Translate timer ticks to microseconds without losing precicision + * + * There are hardware timers whose frequency can't be expressed as + * ticks-per-microsecond integer-value because only a ticks-per-millisecond + * integer-value is precise enough. We don't want to use expensive + * floating-point values here but nonetheless want to translate from ticks + * to time with microseconds precision. Thus, we split the input in two and + * translate both parts separately. This way, we can raise precision by + * shifting the values to their optimal bit position. Afterwards, the + * results are shifted back and merged together again. + * + * Please ensure that the assertion "ticks_per_ms >= 1000" is true + * when calling this method! + */ + template + RESULT_T timer_ticks_to_us(RESULT_T const ticks, + TICS_PER_MS_T const ticks_per_ms) + { + enum { + HALF_WIDTH = (sizeof(RESULT_T) << 2), + MSB_MASK = ~0UL << HALF_WIDTH, + LSB_MASK = ~0UL >> HALF_WIDTH, + MSB_RSHIFT = 10, + LSB_LSHIFT = HALF_WIDTH - MSB_RSHIFT, + }; + RESULT_T const msb = ((((ticks & MSB_MASK) >> MSB_RSHIFT) + * 1000) / ticks_per_ms) << MSB_RSHIFT; + RESULT_T const lsb = ((((ticks & LSB_MASK) << LSB_LSHIFT) + * 1000) / ticks_per_ms) >> LSB_LSHIFT; + return msb + lsb; + } +} + +#endif /* _DRIVERS__TIMER__UTIL_H_ */ diff --git a/repos/os/src/drivers/timer/pit/time_source.cc b/repos/os/src/drivers/timer/pit/time_source.cc index 238457daa..8986b5049 100644 --- a/repos/os/src/drivers/timer/pit/time_source.cc +++ b/repos/os/src/drivers/timer/pit/time_source.cc @@ -12,6 +12,9 @@ * under the terms of the GNU Affero General Public License version 3. */ +/* Genode includes */ +#include + /* local includes */ #include @@ -89,29 +92,8 @@ Duration Timer::Time_source::curr_time() ticks = PIT_MAX_COUNT + 1 - curr_counter; } - /* - * If we would do the translation with one division and - * multiplication over the whole argument, we would loose - * microseconds granularity although the timer frequency would - * allow for such granularity. Thus, we treat the most significant - * half and the least significant half of the argument separate. - * Each half is shifted to the best bit position for the - * translation, then translated, and then shifted back. - */ - using time_t = unsigned long; static_assert(PIT_TICKS_PER_MSEC >= 1000, "Bad TICS_PER_MS value"); - enum { - HALF_WIDTH = (sizeof(time_t) << 2), - MSB_MASK = ~0UL << HALF_WIDTH, - LSB_MASK = ~0UL >> HALF_WIDTH, - MSB_RSHIFT = 10, - LSB_LSHIFT = HALF_WIDTH - MSB_RSHIFT, - }; - time_t const msb = ((((ticks & MSB_MASK) >> MSB_RSHIFT) - * 1000) / PIT_TICKS_PER_MSEC) << MSB_RSHIFT; - time_t const lsb = ((((ticks & LSB_MASK) << LSB_LSHIFT) - * 1000) / PIT_TICKS_PER_MSEC) >> LSB_LSHIFT; - _curr_time_us += (msb + lsb); + _curr_time_us += timer_ticks_to_us(ticks, PIT_TICKS_PER_MSEC); /* use current counter as the reference for the next update */ _counter_init_value = curr_counter;