qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH] hw/timer: Add value matching support to aspeed_ti


From: Cédric Le Goater
Subject: Re: [Qemu-arm] [PATCH] hw/timer: Add value matching support to aspeed_timer
Date: Sun, 5 Jun 2016 19:22:34 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.8.0

On 06/05/2016 06:20 PM, Joel Stanley wrote:
> On Fri, May 27, 2016 at 12:08 AM, Andrew Jeffery <address@hidden> wrote:
>> Value matching allows Linux to boot with CONFIG_NO_HZ_IDLE=y on the
>> palmetto-bmc machine. Two match registers are provided for each timer.
> 
> Thanks for doing this. We now boot faster in my testing.
> 
>>
>> Signed-off-by: Andrew Jeffery <address@hidden>
> 
> Acked-by: Joel Stanley <address@hidden>

Yes. I should have mentioned it also. This is a must have for the SMC 
patches (soon to come) which let us boot directly for the OpenBMC flash 
images.

Acked-by: Cédric Le Goater <address@hidden>

Thanks

C.
 
>> ---
>>
>> The change pulls out ptimer in favour of the regular timer infrastructure. 
>> As a
>> consequence it implements the conversions between ticks and time which feels 
>> a
>> little tedious. Any comments there would be appreciated.
>>
>>  hw/timer/aspeed_timer.c         | 135 
>> ++++++++++++++++++++++++++++++----------
>>  include/hw/timer/aspeed_timer.h |   6 +-
>>  2 files changed, 105 insertions(+), 36 deletions(-)
>>
>> diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
>> index 4b94808821b4..905de0f788b2 100644
>> --- a/hw/timer/aspeed_timer.c
>> +++ b/hw/timer/aspeed_timer.c
>> @@ -9,13 +9,12 @@
>>   * the COPYING file in the top-level directory.
>>   */
>>
>> +#include <math.h>
>>  #include "qemu/osdep.h"
>> -#include "hw/ptimer.h"
>>  #include "hw/sysbus.h"
>>  #include "hw/timer/aspeed_timer.h"
>>  #include "qemu-common.h"
>>  #include "qemu/bitops.h"
>> -#include "qemu/main-loop.h"
>>  #include "qemu/timer.h"
>>  #include "qemu/log.h"
>>  #include "trace.h"
>> @@ -77,21 +76,99 @@ static inline bool timer_can_pulse(AspeedTimer *t)
>>      return t->id >= TIMER_FIRST_CAP_PULSE;
>>  }
>>
>> +static inline bool timer_external_clock(AspeedTimer *t)
>> +{
>> +    return timer_ctrl_status(t, op_external_clock);
>> +}
>> +
>> +static double clock_rates[] = { TIMER_CLOCK_APB_HZ, TIMER_CLOCK_EXT_HZ };
>> +
>> +static inline double calculate_rate(struct AspeedTimer *t)
>> +{
>> +    return clock_rates[timer_external_clock(t)];
>> +}
>> +
>> +static inline double calculate_period(struct AspeedTimer *t)
>> +{
>> +    return NANOSECONDS_PER_SECOND / calculate_rate(t);
>> +}
>> +
>> +static inline uint32_t calculate_ticks(struct AspeedTimer *t, uint64_t 
>> now_ns)
>> +{
>> +    uint64_t delta_ns = now_ns - MIN(now_ns, t->start);
>> +    uint32_t ticks = (uint32_t) floor(delta_ns / calculate_period(t));
>> +
>> +    return t->reload - MIN(t->reload, ticks);
>> +}
>> +
>> +static inline uint64_t calculate_time(struct AspeedTimer *t, uint32_t ticks)
>> +{
>> +    uint64_t delta_ns;
>> +
>> +    ticks = MIN(t->reload, ticks);
>> +    delta_ns = (uint64_t) floor((t->reload - ticks) * calculate_period(t));
>> +
>> +    return t->start + delta_ns;
>> +}
>> +
>> +static uint64_t calculate_next(struct AspeedTimer *t)
>> +{
>> +    uint64_t now;
>> +    uint64_t next;
>> +    int i;
>> +    /* We don't know the relationship between the values in the match
>> +     * registers, so sort using MAX/MIN/zero. We sort in that order as the
>> +     * timer counts down to zero. */
>> +    uint64_t seq[] = {
>> +        MAX(t->match[0], t->match[1]),
>> +        MIN(t->match[0], t->match[1]),
>> +        0,
>> +    };
>> +
>> +    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> +    for (i = 0; i < ARRAY_SIZE(seq); i++) {
>> +        next = calculate_time(t, seq[i]);
>> +        if (now < next) {
>> +            return next;
>> +        }
>> +    }
>> +
>> +    {
>> +        uint64_t reload_ns;
>> +
>> +        reload_ns = (uint64_t) floor(t->reload * calculate_period(t));
>> +        t->start = now - ((now - t->start) % reload_ns);
>> +    }
>> +
>> +    return calculate_next(t);
>> +}
>> +
>>  static void aspeed_timer_expire(void *opaque)
>>  {
>>      AspeedTimer *t = opaque;
>> +    bool interrupt = false;
>> +    uint32_t ticks;
>>
>> -    /* Only support interrupts on match values of zero for the moment - 
>> this is
>> -     * sufficient to boot an aspeed_defconfig Linux kernel.
>> -     *
>> -     * TODO: matching on arbitrary values (see e.g. hw/timer/a9gtimer.c)
>> -     */
>> -    bool match = !(t->match[0] && t->match[1]);
>> -    bool interrupt = timer_overflow_interrupt(t) || match;
>> -    if (timer_enabled(t) && interrupt) {
>> +    if (!timer_enabled(t)) {
>> +        return;
>> +    }
>> +
>> +    ticks = calculate_ticks(t, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>> +
>> +    if (!ticks) {
>> +        interrupt = timer_overflow_interrupt(t) || !t->match[0] || 
>> !t->match[1];
>> +    } else if (ticks <= MIN(t->match[0], t->match[1])) {
>> +        interrupt = true;
>> +    } else if (ticks <= MAX(t->match[0], t->match[1])) {
>> +        interrupt = true;
>> +    }
>> +
>> +    if (interrupt) {
>>          t->level = !t->level;
>>          qemu_set_irq(t->irq, t->level);
>>      }
>> +
>> +    timer_mod(&t->timer, calculate_next(t));
>>  }
>>
>>  static uint64_t aspeed_timer_get_value(AspeedTimer *t, int reg)
>> @@ -100,7 +177,7 @@ static uint64_t aspeed_timer_get_value(AspeedTimer *t, 
>> int reg)
>>
>>      switch (reg) {
>>      case TIMER_REG_STATUS:
>> -        value = ptimer_get_count(t->timer);
>> +        value = calculate_ticks(t, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>>          break;
>>      case TIMER_REG_RELOAD:
>>          value = t->reload;
>> @@ -160,24 +237,21 @@ static void 
>> aspeed_timer_set_value(AspeedTimerCtrlState *s, int timer, int reg,
>>      switch (reg) {
>>      case TIMER_REG_STATUS:
>>          if (timer_enabled(t)) {
>> -            ptimer_set_count(t->timer, value);
>> +            uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> +            int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
>> now);
>> +
>> +            t->start += delta * calculate_period(t);
>> +            timer_mod(&t->timer, calculate_next(t));
>>          }
>>          break;
>>      case TIMER_REG_RELOAD:
>>          t->reload = value;
>> -        ptimer_set_limit(t->timer, value, 1);
>>          break;
>>      case TIMER_REG_MATCH_FIRST:
>>      case TIMER_REG_MATCH_SECOND:
>> -        if (value) {
>> -            /* Non-zero match values are unsupported. As such an interrupt 
>> will
>> -             * always be triggered when the timer reaches zero even if the
>> -             * overflow interrupt control bit is clear.
>> -             */
>> -            qemu_log_mask(LOG_UNIMP, "%s: Match value unsupported by 
>> device: "
>> -                    "0x%" PRIx32 "\n", __func__, value);
>> -        } else {
>> -            t->match[reg - 2] = value;
>> +        t->match[reg - 2] = value;
>> +        if (timer_enabled(t)) {
>> +            timer_mod(&t->timer, calculate_next(t));
>>          }
>>          break;
>>      default:
>> @@ -196,21 +270,16 @@ static void aspeed_timer_ctrl_enable(AspeedTimer *t, 
>> bool enable)
>>  {
>>      trace_aspeed_timer_ctrl_enable(t->id, enable);
>>      if (enable) {
>> -        ptimer_run(t->timer, 0);
>> +        t->start = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> +        timer_mod(&t->timer, calculate_next(t));
>>      } else {
>> -        ptimer_stop(t->timer);
>> -        ptimer_set_limit(t->timer, t->reload, 1);
>> +        timer_del(&t->timer);
>>      }
>>  }
>>
>>  static void aspeed_timer_ctrl_external_clock(AspeedTimer *t, bool enable)
>>  {
>>      trace_aspeed_timer_ctrl_external_clock(t->id, enable);
>> -    if (enable) {
>> -        ptimer_set_freq(t->timer, TIMER_CLOCK_EXT_HZ);
>> -    } else {
>> -        ptimer_set_freq(t->timer, TIMER_CLOCK_APB_HZ);
>> -    }
>>  }
>>
>>  static void aspeed_timer_ctrl_overflow_interrupt(AspeedTimer *t, bool 
>> enable)
>> @@ -351,12 +420,10 @@ static const MemoryRegionOps aspeed_timer_ops = {
>>
>>  static void aspeed_init_one_timer(AspeedTimerCtrlState *s, uint8_t id)
>>  {
>> -    QEMUBH *bh;
>>      AspeedTimer *t = &s->timers[id];
>>
>>      t->id = id;
>> -    bh = qemu_bh_new(aspeed_timer_expire, t);
>> -    t->timer = ptimer_init(bh);
>> +    timer_init_ns(&t->timer, QEMU_CLOCK_VIRTUAL, aspeed_timer_expire, t);
>>  }
>>
>>  static void aspeed_timer_realize(DeviceState *dev, Error **errp)
>> @@ -404,7 +471,7 @@ static const VMStateDescription vmstate_aspeed_timer = {
>>      .fields = (VMStateField[]) {
>>          VMSTATE_UINT8(id, AspeedTimer),
>>          VMSTATE_INT32(level, AspeedTimer),
>> -        VMSTATE_PTIMER(timer, AspeedTimer),
>> +        VMSTATE_TIMER(timer, AspeedTimer),
>>          VMSTATE_UINT32(reload, AspeedTimer),
>>          VMSTATE_UINT32_ARRAY(match, AspeedTimer, 2),
>>          VMSTATE_END_OF_LIST()
>> diff --git a/include/hw/timer/aspeed_timer.h 
>> b/include/hw/timer/aspeed_timer.h
>> index 44dc2f89d5c6..970fea83143c 100644
>> --- a/include/hw/timer/aspeed_timer.h
>> +++ b/include/hw/timer/aspeed_timer.h
>> @@ -22,7 +22,8 @@
>>  #ifndef ASPEED_TIMER_H
>>  #define ASPEED_TIMER_H
>>
>> -#include "hw/ptimer.h"
>> +#include "qemu/typedefs.h"
>> +#include "qemu/timer.h"
>>
>>  #define ASPEED_TIMER(obj) \
>>      OBJECT_CHECK(AspeedTimerCtrlState, (obj), TYPE_ASPEED_TIMER);
>> @@ -33,15 +34,16 @@ typedef struct AspeedTimer {
>>      qemu_irq irq;
>>
>>      uint8_t id;
>> +    QEMUTimer timer;
>>
>>      /**
>>       * Track the line level as the ASPEED timers implement edge triggered
>>       * interrupts, signalling with both the rising and falling edge.
>>       */
>>      int32_t level;
>> -    ptimer_state *timer;
>>      uint32_t reload;
>>      uint32_t match[2];
>> +    uint64_t start;
>>  } AspeedTimer;
>>
>>  typedef struct AspeedTimerCtrlState {
>> --
>> 2.7.4
>>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]