Tickless mode fixes #13

diggit · 2020-04-16T13:08:10Z

This is initial effort to fix timing issues in Argon, more info in #11. I am open open to suggestions.

flit · 2020-04-19T17:22:00Z

Started reviewing… Thanks! 😄

flit · 2020-04-19T17:25:36Z

src/ar_kernel.cpp

@@ -290,7 +259,8 @@ bool ar_kernel_increment_tick_count(unsigned ticks)
            ar_list_node_t * next = node->m_next;

            // Is it time to wake this thread?
-            if (g_ar.tickCount >= thread->m_wakeupTime)
+            //TODO: would it be safe to cache tick_count?


I was wondering about that too. It's probably safe to cache it within a single function like this one, or ar_kernel_scheduler().

flit · 2020-04-19T17:27:38Z

src/ar_kernel.cpp

-    return g_ar.tickCount * ar_get_milliseconds_per_tick();
-#endif // AR_ENABLE_TICKLESS_IDLE
+    return ar_port_get_time_absolute_ms();
+    // return ar_port_get_time_absolute_ticks() * kSchedulerQuanta_ms; //do we have to limit resolution?


Imo, it's better if resolution is not limited.

Ok, I was not sure about consequences. Will Fix.

flit · 2020-04-19T17:29:40Z

src/ar_kernel.cpp

 uint32_t ar_get_tick_count(void)
 {
-#if AR_ENABLE_TICKLESS_IDLE
-    uint32_t elapsed_ticks = ar_port_get_timer_elapsed_us() / (kSchedulerQuanta_ms * 1000);
-    return g_ar.tickCount + elapsed_ticks;
-#else
-    return g_ar.tickCount;
-#endif // AR_ENABLE_TICKLESS_IDLE
+    return ar_port_get_time_absolute_ticks();
 }

 // See ar_kernel.h for documentation of this function.
 uint32_t ar_get_millisecond_count(void)
 {
-#if AR_ENABLE_TICKLESS_IDLE
-    uint32_t elapsed_ms = ar_port_get_timer_elapsed_us() / 1000;
-    return g_ar.tickCount * ar_get_milliseconds_per_tick() + elapsed_ms;
-#else
-    return g_ar.tickCount * ar_get_milliseconds_per_tick();
-#endif // AR_ENABLE_TICKLESS_IDLE
+    return ar_port_get_time_absolute_ms();
+    // return ar_port_get_time_absolute_ticks() * kSchedulerQuanta_ms; //do we have to limit resolution?


ar_get_tick_count() and ar_get_millisecond_count() should probably be inline. That does mean exposing some port APIs publicly…

Or can we just completely remove them? Is there any reason to not directly call ar_port_get_time_absolute_<x>()? (Renamed?)

Yeah, forgot to inline them. Even though, compiler would probably inline them anyway.
Well, I think that application should have access to system time. Also application should not have access to port functions.

flit · 2020-04-19T17:31:45Z

src/cortex_m/ar_port_f303_chained.cpp

 }

+//TODO: is return type sufficient?
+uint32_t ar_port_get_time_absolute_ms()


Probably not, need uint64_t here.

flit · 2020-04-19T17:33:16Z

src/cortex_m/ar_port_f303_chained.cpp

+//TODO: move ar_get_microseconds() somewhere else and redirect to ar_port_get_time_absolute_us() ?
 WEAK uint64_t ar_get_microseconds()


Yeah, move it next to the other get-time APIs.

flit

I like the changes! Thanks again. 👍

The only thing I see that really needs to be done is to split the timer-porting code from the rest of the Cortex-M porting code, so we don't have duplicates.

diggit · 2020-04-27T14:33:18Z

I see no problem i splitting them. Any idea about port sources structure? Should there be some automation in what is compiled (eg through ar_config) or leave it up to user, which sources to compile and link?

What about:

port/
- port.hpp (common header defining interface to port functions)
- arch/
  - cortex/
    - cm0.cpp
    - cm3_4_7.cpp (split if needed)
- target/
  - stm32/
    - f303.cpp
    - f446.cpp
    - 103.cpp
  - nxp/
    - kinetis_??.cpp

This change allows to utilize HW timers more easily. (default ar_port.cpp using systick needs updating)

*Single* uses only one timer as microsecond counter between ticks. *Chained* uses two chained timer s (on hardware level) to count microseconds between tics and ticks themselves. Most precise timing with this solution.

- some f303 port cleanups - add port using generic ARM SysTick with explained limitations only lightly tested!

flit · 2020-07-06T21:48:04Z

My sincerest apologies for being quiet for too long! 🙏

For the port sources, I guess I'm not sure what the best solution is. One of the goals I had for the Argon repo was for it to be usable as a git submodule, without including too much ancillary code you don't need. (I've used it like this in projects; it's nice.)

Options:

Your proposal with device support in the core repo.
The ideal solution would be dependency-managed packages, but I think the C/C++ package manager space is too messy (certainly compared to Rust, for instance).
Separate each device-specific port into its own repo that you would submodule next to the core Argon repo.
Have device support grouped into another repo from which users can just pick out the file(s) they need and copy into their project. Or you could submodule it if you don't care about extra files.

For now I'm leaning towards your proposal. 😄 I can live with a relatively small number of device-support files in the core repo. And it can always be changed if needed.

As for builds, I've never been a fan of using macros to include/exclude files in the build. So I'd rather leave it up to the user.

flit reviewed Apr 19, 2020

View reviewed changes

flit approved these changes Apr 19, 2020

View reviewed changes

diggit added 3 commits May 3, 2020 00:26

Move all ticking functionality into ar_port

c426cac

This change allows to utilize HW timers more easily. (default ar_port.cpp using systick needs updating)

add ar_port examples for STM32F303 HW timers

060ce54

*Single* uses only one timer as microsecond counter between ticks. *Chained* uses two chained timer s (on hardware level) to count microseconds between tics and ticks themselves. Most precise timing with this solution.

add systick example port and cleanups

da2b0b3

- some f303 port cleanups - add port using generic ARM SysTick with explained limitations only lightly tested!

diggit force-pushed the ticklessFixes branch from 30f23d7 to da2b0b3 Compare May 2, 2020 22:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tickless mode fixes #13

Tickless mode fixes #13

diggit commented Apr 16, 2020 •

edited

Loading

flit commented Apr 19, 2020

flit Apr 19, 2020

flit Apr 19, 2020

diggit Apr 27, 2020

flit Apr 19, 2020 •

edited

Loading

diggit Apr 27, 2020

flit Apr 19, 2020

flit Apr 19, 2020

flit left a comment

diggit commented Apr 27, 2020

flit commented Jul 6, 2020

		//TODO: move ar_get_microseconds() somewhere else and redirect to ar_port_get_time_absolute_us() ?
		WEAK uint64_t ar_get_microseconds()

Tickless mode fixes #13

Are you sure you want to change the base?

Tickless mode fixes #13

Conversation

diggit commented Apr 16, 2020 • edited Loading

flit commented Apr 19, 2020

flit Apr 19, 2020

Choose a reason for hiding this comment

flit Apr 19, 2020

Choose a reason for hiding this comment

diggit Apr 27, 2020

Choose a reason for hiding this comment

flit Apr 19, 2020 • edited Loading

Choose a reason for hiding this comment

diggit Apr 27, 2020

Choose a reason for hiding this comment

flit Apr 19, 2020

Choose a reason for hiding this comment

flit Apr 19, 2020

Choose a reason for hiding this comment

flit left a comment

Choose a reason for hiding this comment

diggit commented Apr 27, 2020

flit commented Jul 6, 2020

diggit commented Apr 16, 2020 •

edited

Loading

flit Apr 19, 2020 •

edited

Loading