timer: Align SOF timer API with Zephyr

[softwarecki]

The purpose of this PR is to prepare the SOF to use zephyr timer API.

Added functions platform_timer_ticks_to_ms and platform_timer_ticks_to_us which allows convert the time expressed in ticks to seconds.

Waiting for a certain time has been simplified by adding a functions wait_delay_ms and wait_delay_us. Previously now, its required a conversion between time to ticks using clock_ms_to_ticks.

New functions platform_timer_timeout_calc_ms and platform_timer_timeout_calc_us simplify the timeout calculation.

[lgirdwood]

@kkarask partial CI result, are we good to merge ?

[lgirdwood]

SOFCI TEST

[wszypelt]

@lgirdwood We had a problem with one machine, the rerun is already going, after it it will be known if everything is ok.

[lyakh]

Sorry, maybe you could sketch approximately what next steps would be for the migration to the Zephyr timer API, maybe then it would be easier to understand where these changes are leading to?

[lgirdwood]

@softwarecki lets also try and see how we can partition the xtos related APIs (as a code move) to src/arch/xtensa/xtos/. This way we can keep the generic code space clean. This could be a separate PR though, lets discuss it tomorrow.

[lgirdwood]

platform_timer_timeout_calc_ms -> sys_clock_timeout_end_calc(K_MSEC(ms));
platform_timer_timeout_calc_us -> sys_clock_timeout_end_calc(K_USEC(us));
platform_timer_ticks_to_ms -> k_ticks_to_ms_near64
platform_timer_ticks_to_us -> k_ticks_to_us_near64
platform_timer_get -> sys_clock_tick_get
platform_timer_set -> k_timer_start(K_TICKS(ticks - sys_clock_tick_get()))
arch_timer_get_system -> k_cycle_get_64
arch_timer_set -> platform_timer_set -> k_timer_start(K_CYCLES(cycles - k_cycle_get_64())

@softwarecki can we not use some of these directly in this PR and create a wrapper for xtos (that calls the old APIs) ? This should simplify some of the other updates too (since they will be native APIs and be near xtos directory).

[lgirdwood]

Intel and MTK (and possible others) have 64bit timers alongside the xtensa CCOUNT 32bit timer. The Zephyr timer API is mostly 32bit today, but it uses the 64bit timer driver on Intel. I think there may be plans for full 64bit timer API on Zephyr @andyross ?.

[lgirdwood]

Intel and MTK (and possible others) have 64bit timers alongside the xtensa CCOUNT 32bit timer. The Zephyr timer API is mostly 32bit today, but it uses the 64bit timer driver on Intel. I think there may be plans for full 64bit timer API on Zephyr @andyross ?.

ok, ignore me ;) Things are moving fast, I think it fully 64 bit now, but I don't know if that also includes other APIs like scheduler that use time.

[hlleng]

how do you know it is 32bit? AFAIK there is a cycle counter which spreads over 2 x 32 bit registers.

i see the CCOUNT register is 32bit
.

what timer does imx8mp use?

i use xtensa_sys_timer.c now

[dbaluta]

I see. I think SOF / Zephyr uses 64 bit timers. if CCOUNT overflows then we increment the high 32 bits.

See: sof/src/arch/xtensa/drivers/timer.c arch_timer_get_system

[hlleng]

Yes, but when i use Zephyr , it can not include sof/src/arch/xtensa/drivers/timer.c.

So i think we should update xtensa_sys_timer.c this file, and add sys_clock_cycle_get_32, right ?

[softwarecki]

@lenghonglin you can use k_cycle_get_32

[dbaluta]

I understand your concern, and having a second look at it i might have my doubts too.

Did you see any problems with the current implementation?

For what I understand SOF generic layer uses k_cycle_get_64 and that's fine. But we need to understand how does that translates for IMX.

Looking at zephyr: include/arch/xtensa/arch/arch.h we have:

static inline uint64_t arch_k_cycle_get_64(void)
{
»       return sys_clock_cycle_get_64();
}

but I couldn't find the implementation of sys_clock_cycle_get_64. It must be there because the code compiles.

I don't I understand what you are suggesting with adding sys_clock_cycle_get_32

[hlleng]

I understand your concern, and having a second look at it i might have my doubts too.

Did you see any problems with the current implementation?

For what I understand SOF generic layer uses k_cycle_get_64 and that's fine. But we need to understand how does that translates for IMX.

Looking at zephyr: include/arch/xtensa/arch/arch.h we have:

static inline uint64_t arch_k_cycle_get_64(void)
{
»       return sys_clock_cycle_get_64();
}

but I couldn't find the implementation of sys_clock_cycle_get_64. It must be there because the code compiles.

I don't I understand what you are suggesting with adding sys_clock_cycle_get_32

Yes, it can be compiled, and I couldn't find the implementation of sys_clock_cycle_get_64 too,

[hlleng]

Intel and MTK (and possible others) have 64bit timers alongside the xtensa CCOUNT 32bit timer. The Zephyr timer API is mostly 32bit today, but it uses the 64bit timer driver on Intel. I think there may be plans for full 64bit timer API on Zephyr @andyross ?.

ok, ignore me ;) Things are moving fast, I think it fully 64 bit now, but I don't know if that also includes other APIs like scheduler that use time.

There is a question, see zephyrproject-rtos/zephyr#39935 this PR, though zephyr support 64 bit timer api , but the most important is the hardware should support 64bit timer register, or it should not be work.

For xtensa_timer, It not support TIMER_HAS_64BIT_CYCLE_COUNTER https://github.com/zephyrproject-rtos/zephyr/blob/918a574c88c3b06cfbb2c6e6382b15482834b647/drivers/timer/Kconfig#L265

[hlleng]

@lenghonglin you can use k_cycle_get_32

Yes, I think should be support both k_cycle_get_32 and k_cycle_get_64 .

[dbaluta]

@lenghonglin you can use k_cycle_get_32

Yes, but the common SOF code uses k_cycle_get_64 and it looks like this translates into nothing for IMX. Am i right, @lenghonglin

[hlleng]

@lenghonglin you can use k_cycle_get_32

Yes, but the common SOF code uses k_cycle_get_64 and it looks like this translates into nothing for IMX. Am i right, @lenghonglin

1. what kind of timer does IMX used with Zephyr? Relate to Konfig
2. ccount width is 32bit or 64bit?

[paulstelian97]

@lenghonglin you can use k_cycle_get_32

Yes, but the common SOF code uses k_cycle_get_64 and it looks like this translates into nothing for IMX. Am i right, @lenghonglin

1. what kind of timer does IMX used with Zephyr? Relate to Konfig

We are looking into it, right now we don't know yet.

2. ccount width is 32bit or 64bit?

I believe, but still need to double check, that it is 32-bit.

What kind of issues do you see/predict right now?

[hlleng]

@lenghonglin you can use k_cycle_get_32

Yes, but the common SOF code uses k_cycle_get_64 and it looks like this translates into nothing for IMX. Am i right, @lenghonglin

1. what kind of timer does IMX used with Zephyr? Relate to Konfig

We are looking into it, right now we don't know yet.

2. ccount width is 32bit or 64bit?

I believe, but still need to double check, that it is 32-bit.

What kind of issues do you see/predict right now?

I mean if the hardware not support TIMER_HAS_64BIT_CYCLE_COUNTER , and sof call k_cycle_get_64, so it will return 0
see this pic

[andyross]

Sorry, just catching up here.

To answer the questions above: Zephyr's app-facing timeout API is selectable via CONFIG_TIMER_64BIT. If that is set to y (it's on by default), then all kernel timeouts are stored internally in a 64 bit count of ticks.

The Zephyr driver interface remains mostly 32 bit internally, meaning that the longest delay between any two announced ticks must fit in a signed 32 bit integer. There hasn't been desire to augment this on any current platforms (rollover on the 38.4 MHz clock would be almost a full minute, for example), but if there's desire for that it would be easy to do.

Finally the hardware cycle API, which is on top of the driver but parallel to the timeout system, is required to be only 32 bit. Recently we've seen drivers that want to expose the full counter via k_cycle_get_64(), which is what CONFIG_TIMER_HAS_64BIT_CYCLE_COUNTER is for. The cavs_timer driver for the HDA wall clock supports that just fine too.

[hlleng]

Yes, for support 32bit timer, I think it should better support k_cycle_get_32

[dbaluta]

@andyross @lenghonglin OK, Xtensa hifi4 only has 32bit timer register and for calls to k_cycle_get_64 Zephyr implementation just simply returns zero.

Would it be possible to add support for k_cycle_get_64 by just keeping track of when ccount register overflows?

e.g this implementation is used with SOF.

uint64_t arch_timer_get_system(struct timer *timer)
{
»       uint64_t time = 0;
»       uint32_t flags;
»       uint32_t low;
»       uint32_t high;
»       uint32_t ccompare;

»       if (timer->id >= ARCH_TIMER_COUNT)
»       »       goto out;

»       ccompare = xthal_get_ccompare(timer->id);

»       flags = arch_interrupt_global_disable();

»       /* read low 32 bits */
»       low = xthal_get_ccount();

»       /* check and see whether 32bit IRQ is pending for timer */
»       if (arch_interrupt_get_status() & (1 << timer->irq) && ccompare == 1) {
»       »       /* yes, overflow has occured but handler has not run */
»       »       high = timer->hitime + 1;
»       } else {
»       »       /* no overflow */
»       »       high = timer->hitime;
»       }

»       time = ((uint64_t)high << 32) | low;

»       arch_interrupt_global_enable(flags);

out:

»       return time;
}

SOF core code assumes that k_cycle_get_64 works for all platforms.

[hlleng]

@andyross @lenghonglin OK, Xtensa hifi4 only has 32bit timer register and for calls to k_cycle_get_64 Zephyr implementation just simply returns zero.

Would it be possible to add support for k_cycle_get_64 by just keeping track of when ccount register overflows?

e.g this implementation is used with SOF.

uint64_t arch_timer_get_system(struct timer *timer)
{
»       uint64_t time = 0;
»       uint32_t flags;
»       uint32_t low;
»       uint32_t high;
»       uint32_t ccompare;

»       if (timer->id >= ARCH_TIMER_COUNT)
»       »       goto out;

»       ccompare = xthal_get_ccompare(timer->id);

»       flags = arch_interrupt_global_disable();

»       /* read low 32 bits */
»       low = xthal_get_ccount();

»       /* check and see whether 32bit IRQ is pending for timer */
»       if (arch_interrupt_get_status() & (1 << timer->irq) && ccompare == 1) {
»       »       /* yes, overflow has occured but handler has not run */
»       »       high = timer->hitime + 1;
»       } else {
»       »       /* no overflow */
»       »       high = timer->hitime;
»       }

»       time = ((uint64_t)high << 32) | low;

»       arch_interrupt_global_enable(flags);

out:

»       return time;
}

SOF core code assumes that k_cycle_get_64 works for all platforms.

This way maybe cause confusion for developer.
I think it's better

#if defined(__ZEPHYR__)

#ifdef TIMER_HAS_64BIT_CYCLE_COUNTER 
    #define sof_timer k_cycle_get_64
#else
    #define sof_timer k_cycle_get_32
#endif

#endif

[lgirdwood]

This way maybe cause confusion for developer. I think it's better

#if defined(__ZEPHYR__)

#ifdef TIMER_HAS_64BIT_CYCLE_COUNTER 
    #define sof_timer k_cycle_get_64
#else
    #define sof_timer k_cycle_get_32
#endif

#endif

@lenghonglin we should not be using any native SOF API call for RTOS functions (like timers) but use Zephyr conditional logic (at build time or runtime) like you have above to get the correct Zephyr native timer call.