You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "jlaitine (via GitHub)" <gi...@apache.org> on 2025/06/04 15:13:22 UTC

[PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

jlaitine opened a new pull request, #16485:
URL: https://github.com/apache/nuttx/pull/16485

   
   ## Summary
   
   Add up_perf_ functions for MPFS, which don't rely on alarm/oneshot interface.
   
   Also add optimized up_udelay and up_ndelay functions, which use the MTIMER directly to measure time; making them accurate and more multithreading friendly.
   
   ## Impact
   
   Only for MPFS platform;
   - makes up_perf_gettime faster, when the call doesn't go through the alarm framework. which in turns helps profiling
   - uses the 1MHz MTIME counter on MPFS to directly measure up_udelay, preventing unnecessary delays when call is pre-empted by another task or interrupt.
   
   ## Testing
   
   Tested on a custom MPFS250T based board & segger system view tool to compare the overhead  with and without this patch.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #16485:
URL: https://github.com/apache/nuttx/pull/16485


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128427950


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I hear and understand you, but I don't think what you claim is *strictly* true. Also the very first example in your link ("For example, the following code forces CONSOLE to y whenever USB_CONSOLE is y", selects "CONSOLE") selects a flag with a prompt string.
   
   So it works.
   
   I do agree that in this case there are just the following pitfalls:
   - If user turns off the ARCH_PERF_EVENTS in .config, he will create a configuration which doesn't compile.
   - If user tries to define the ARCH_PERF_EVENTS to something else than "y", it will just fail (if left undefined, it will be selected anyhow in .config, if set to "n", there will most likely be an error).
   
   It wouldn't get any better if I just defined it to "y" in every defconfig, it would still be possible to user to turn in off to create a broken configuration.
   
   Nevertheless, I'll try to fix it in the PR above, cleaning up all the places for the macros in question..
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128126312


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   Kconfig with prompt string is designed to be enabled by defconfig, only Kconfig without prompt string must and should be selected in Kconfig.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128016198


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I'll try to refine the solution like this: https://github.com/apache/nuttx/pull/16487
   
   I believe this fixes your concern (if it passes the CI ...)
   
   Btw. I am not quite happy how the high resolution hardware timer and arch_alarm and oneshot are tied together, and also perf functions being there as well. Architecturally, IMHO, it would be cleaner if:
   
   - An architecture specific timer interface, supporting reading a HW timer; something like clock_t "arch_timer_get(int timer_id)", "arch_timer_freq(int timer_id)" and functions to set register interrupt cb for absolute and relative timeout time (one registration per timer). This is something that every architecture supports anyhow, currently for a single timer. (this is probably what "oneshot" now tries to be...)
   
   - Alarm interface should be a client for a single arch_timers, supporting a queue of alarms. Any kernel driver could just register an alarm (with relative and/or absolute time). There could be also a connection to power management/wakeup support for calendar alarms from here.
   
   - up_perf_* functions would be just defined to arch_timer_get, arch_timer_freq...
   - Watchdogs would just register an alarm to alarm interface
   - Systick would just register an alarm to alarm interface
   
   This would allow registering precise alarm callbacks also for ticked systems, and harmonize the functionality between systicked/tickless architectures.
   



##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I'll try to refine the solution like this: https://github.com/apache/nuttx/pull/16487
   
   I believe this fixes your concern (if it passes the CI ...)
   
   Btw. I am not quite happy how the high resolution hardware timer and arch_alarm and oneshot are tied together, and also perf functions being there as well. Architecturally, IMHO, it would be cleaner if:
   
   - An architecture specific timer interface, supporting reading a HW timer; something like clock_t "arch_timer_get(int timer_id)", "arch_timer_freq(int timer_id)" and functions to set register interrupt cb for absolute and relative timeout time (one registration per timer). This is something that every architecture supports anyhow, currently for a single timer. (this is probably what "oneshot" now tries to be...)
   
   - Alarm interface should be a client for a single arch_timer, supporting a queue of alarms. Any kernel driver could just register an alarm (with relative and/or absolute time). There could be also a connection to power management/wakeup support for calendar alarms from here.
   
   - up_perf_* functions would be just defined to arch_timer_get, arch_timer_freq...
   - Watchdogs would just register an alarm to alarm interface
   - Systick would just register an alarm to alarm interface
   
   This would allow registering precise alarm callbacks also for ticked systems, and harmonize the functionality between systicked/tickless architectures.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128127913


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   please see: https://docs.zephyrproject.org/latest/build/kconfig/tips.html#select-statements



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128016198


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I'll try to refine the solution like this: https://github.com/apache/nuttx/pull/16487
   
   I believe this fixes your concern (if it passes the CI ...)
   
   Btw. I am not quite happy how the high resolution hardware timer and arch_alarm and oneshot are tied together, and also perf functions being there as well. Architecturally, IMHO, it would be cleaner if:
   
   - There would be an architecture specific timer interface, supporting reading a HW timer; something like clock_t "arch_timer_get(int timer_id)", "arch_timer_freq(int timer_id)" and functions to register interrupt cb for absolute and relative timeout (one registration per timer). This is something that every architecture supports anyhow, currently for a single timer. (this is probably what "oneshot" now tries to be, but it is mixed with systicks and whatnot...)
   
   - Alarm interface should be a client for a single arch_timer, supporting a queue of alarms. Any kernel driver could just register an alarm (with relative and/or absolute time). There could be also a connection to power management/wakeup support for calendar alarms from here.
   
   - up_perf_* functions would be just defined to arch_timer_get, arch_timer_freq...
   - Watchdogs would just register an alarm to alarm interface
   - Systick would just register an alarm to alarm interface
   
   This would allow registering precise alarm callbacks also for ticked systems, and harmonize the functionality between systicked/tickless architectures.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128016198


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I'll try to refine the solution like this: https://github.com/apache/nuttx/pull/16487
   
   I believe this fixes your concern (if it passes the CI ...)
   
   Btw. I am not quite happy how the high resolution hardware timer and arch_alarm and oneshot are tied together, and also perf functions being there as well. Architecturally, IMHO, it would be cleaner if:
   
   - An architecture specific timer interface, supporting reading a HW timer; something like clock_t "arch_timer_get(int timer_id)", "arch_timer_freq(int timer_id)" and functions to set register interrupt cb for absolute and relative timeout time (one registration per timer). This is something that every architecture supports anyhow, currently for a single timer.
   
   - Alarm interface should support a queue of alarms, where any kernel driver could just register an alarm (with relative and/or absolute time). There could be also a connection to power management/wakeup support for calendar alarms from here.
   
   - up_perf_* functions would be just defined to arch_timer_get, arch_timer_freq...
   - Watchdogs would just register an alarm to alarm interface
   - Systick would just register an alarm to alarm interface
   
   This would allow registering precise alarm callbacks also for ticked systems, and harmonize the functionality between systicked/tickless architectures.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2127895989


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   Why not? I wanted it to be on always for all mpfs targets, and this does that.
   I'll recheck what was the linking error in alarm interface. Basically, I never want the up_perf functions from there on mpfs, since they are already defined for all mpfs targets.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128016198


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I'll try to refine the solution like this: https://github.com/apache/nuttx/pull/16487
   
   I believe this fixes your concern (if it passes the CI ...)
   
   Btw. I am not quite happy how the high resolution hardware timer and arch_alarm and oneshot are tied together, and also perf functions being there as well. Architecturally, IMHO, it would be cleaner if:
   
   - An architecture specific timer interface, supporting reading a HW timer; something like clock_t "arch_timer_get(int timer_id)", "arch_timer_freq(int timer_id)" and functions to set register interrupt cb for absolute and relative timeout time (one registration per timer). This is something that every architecture supports anyhow, currently for a single timer. (this is probably what "oneshot" now tries to be...)
   
   - Alarm interface should support a queue of alarms, where any kernel driver could just register an alarm (with relative and/or absolute time). There could be also a connection to power management/wakeup support for calendar alarms from here.
   
   - up_perf_* functions would be just defined to arch_timer_get, arch_timer_freq...
   - Watchdogs would just register an alarm to alarm interface
   - Systick would just register an alarm to alarm interface
   
   This would allow registering precise alarm callbacks also for ticked systems, and harmonize the functionality between systicked/tickless architectures.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2127788134


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   but ARCH_PERF_EVENTS shouldn't be selected in Kconfig.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128016198


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I'll try to refine the solution like this: https://github.com/apache/nuttx/pull/16487
   
   I believe this fixes your concern (if it passes the CI ...)
   
   Btw. I am not quite happy how the high resolution hardware timer and arch_alarm and oneshot are tied together, and also perf functions being there as well. Architecturally, IMHO, it would be cleaner if:
   
   - There would be an architecture specific timer interface, supporting reading a HW timer; something like clock_t "arch_timer_get(int timer_id)", "arch_timer_freq(int timer_id)" and functions to set register interrupt cb for absolute and relative timeout time (one registration per timer). This is something that every architecture supports anyhow, currently for a single timer. (this is probably what "oneshot" now tries to be, but it is mixed with systicks and whatnot...)
   
   - Alarm interface should be a client for a single arch_timer, supporting a queue of alarms. Any kernel driver could just register an alarm (with relative and/or absolute time). There could be also a connection to power management/wakeup support for calendar alarms from here.
   
   - up_perf_* functions would be just defined to arch_timer_get, arch_timer_freq...
   - Watchdogs would just register an alarm to alarm interface
   - Systick would just register an alarm to alarm interface
   
   This would allow registering precise alarm callbacks also for ticked systems, and harmonize the functionality between systicked/tickless architectures.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#issuecomment-2941071264

   Fixed the mpfs:opensbi build linking. I had to explicitly select ARCH_PERF_EVENTS for all the mpfs targets. Sorry for the hassle...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2127895989


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   Why not? I wanted it to be on always for all mpfs targets, and this does that



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/risc-v/mpfs: Add optimized perf timer functions [nuttx]

Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #16485:
URL: https://github.com/apache/nuttx/pull/16485#discussion_r2128016198


##########
arch/risc-v/Kconfig:
##########
@@ -214,6 +214,8 @@ config ARCH_CHIP_MPFS
 	select ARCH_HAVE_PWM_MULTICHAN
 	select ARCH_HAVE_S_MODE
 	select ARCH_RV_CPUID_MAP
+	select ARCH_HAVE_PERF_EVENTS
+	select ARCH_PERF_EVENTS

Review Comment:
   I'll try to refine the solution like this: https://github.com/apache/nuttx/pull/16487
   
   I believe this fixes your concern (if it passes the CI ...)
   
   Btw. I am not quite happy how the high resolution hardware timer and arch_alarm and oneshot are tied together, and also perf functions being there as well. Architecturally, IMHO, it would be cleaner if:
   
   - There would be an architecture specific timer interface, supporting reading a HW timer; something like clock_t "arch_timer_get(int timer_id)", "arch_timer_freq(int timer_id)" and functions to set register interrupt cb for absolute and relative timeout time (one registration per timer). This is something that every architecture supports anyhow, currently for a single timer. (this is probably what "oneshot" now tries to be...)
   
   - Alarm interface should be a client for a single arch_timer, supporting a queue of alarms. Any kernel driver could just register an alarm (with relative and/or absolute time). There could be also a connection to power management/wakeup support for calendar alarms from here.
   
   - up_perf_* functions would be just defined to arch_timer_get, arch_timer_freq...
   - Watchdogs would just register an alarm to alarm interface
   - Systick would just register an alarm to alarm interface
   
   This would allow registering precise alarm callbacks also for ticked systems, and harmonize the functionality between systicked/tickless architectures.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org