From 287fc97eadd2fd4dc556da0780041a431673c536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Chru=C5=9Bci=C5=84ski?= Date: Mon, 31 Mar 2025 11:56:36 +0200 Subject: [PATCH 1/3] nrfx: hal: nrf_grtc: Remove readback from event clear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Readback was added to ensure that interrupt is not triggered spuriously. It may only happen if event clearing is done just before exiting the interrupt. It is not the case in the GRTC driver. Each access to the register interface is costly (~160 320MHz cycles) so it must be avoided if possible. Signed-off-by: Krzysztof Chruściński --- nrfx/hal/nrf_grtc.h | 1 - 1 file changed, 1 deletion(-) diff --git a/nrfx/hal/nrf_grtc.h b/nrfx/hal/nrf_grtc.h index 87318d4f..58163ba4 100644 --- a/nrfx/hal/nrf_grtc.h +++ b/nrfx/hal/nrf_grtc.h @@ -1559,7 +1559,6 @@ NRF_STATIC_INLINE void nrf_grtc_event_clear(NRF_GRTC_Type * p_reg, nrf_grtc_even #endif *((volatile uint32_t *)((uint8_t *)p_reg + (uint32_t)event)) = 0x0UL; - nrf_event_readback((uint8_t *)p_reg + (uint32_t)event); } #if NRF_GRTC_HAS_RTCOUNTER From c7e1dbfc2604c2d66861b0b109a19b6e1f0eaf56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Chru=C5=9Bci=C5=84ski?= Date: Mon, 31 Mar 2025 12:41:37 +0200 Subject: [PATCH 2/3] nrfx: drivers: grtc: Use INTPEND in the interrupt handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use INTPEND in the interrupt handler to optimize handler execution. Do not read CC value from GRTC registers before calling the user handler. User can read that if it is needed. Reading GRTC registers is costly and should be avoided if possible. Signed-off-by: Krzysztof Chruściński --- nrfx/drivers/src/nrfx_grtc.c | 90 +++++++++++++++++------------------- 1 file changed, 42 insertions(+), 48 deletions(-) diff --git a/nrfx/drivers/src/nrfx_grtc.c b/nrfx/drivers/src/nrfx_grtc.c index cf7d9450..aee64eb5 100644 --- a/nrfx/drivers/src/nrfx_grtc.c +++ b/nrfx/drivers/src/nrfx_grtc.c @@ -896,69 +896,63 @@ nrfx_err_t nrfx_grtc_syscounter_cc_value_read(uint8_t channel, uint64_t * p_val) return err_code; } +#if NRF_GRTC_HAS_RTCOUNTER || (NRFY_GRTC_HAS_EXTENDED && NRFY_GRTC_HAS_SYSCOUNTERVALID) +#define GRTC_EXT 1 +#endif + static void grtc_irq_handler(void) { - uint32_t evt_to_process = GRTC_CHANNEL_MASK_TO_INT_MASK(allocated_channels_mask_get() & - used_channels_mask_get()); -#if NRF_GRTC_HAS_RTCOUNTER - evt_to_process |= (GRTC_NON_SYSCOMPARE_INT_MASK & ~NRF_GRTC_INT_SYSCOUNTERVALID_MASK); -#endif - uint32_t event_mask = nrfy_grtc_events_process(NRF_GRTC, evt_to_process); - uint32_t active_int_mask = nrfy_grtc_int_enable_check(NRF_GRTC, event_mask); - nrf_grtc_event_t event; + uint32_t intpend = nrf_grtc_int_pending_get(NRF_GRTC); - for (uint32_t i = 0; i < NRFX_GRTC_CONFIG_NUM_OF_CC_CHANNELS; i++) - { - uint8_t channel = m_cb.channel_data[i].channel; + while (intpend) { + uint32_t idx = 31 - NRFX_CLZ(intpend); + + intpend &= ~NRFX_BIT(idx); + nrf_grtc_event_clear(NRF_GRTC, NRFY_INT_BITPOS_TO_EVENT(idx)); - event = nrfy_grtc_sys_counter_compare_event_get(channel); - if (active_int_mask & NRFY_EVENT_TO_INT_BITMASK(event)) + if (!NRFX_IS_ENABLED(GRTC_EXT) || idx < 16) { - NRFX_LOG_INFO("Event: NRF_GRTC_EVENT_COMPARE_%d.", channel); - if (m_cb.channel_data[i].handler) + for (uint32_t i = 0; i < NRFX_GRTC_CONFIG_NUM_OF_CC_CHANNELS; i++) { - m_cb.channel_data[i].handler((int32_t)channel, - nrfy_grtc_sys_counter_cc_get(NRF_GRTC, channel), - m_cb.channel_data[i].p_context); + if ((m_cb.channel_data[i].channel == idx) && m_cb.channel_data[i].handler) + { + m_cb.channel_data[i].handler(idx, m_cb.channel_data[i].p_context); + break; + } + } + /* Return early as this is the most likely scenario (single CC expiring). */ + if (intpend == 0) + { + break; } } - } #if NRF_GRTC_HAS_RTCOUNTER - if (active_int_mask & NRF_GRTC_INT_RTCOMPARE_MASK) - { - NRFX_LOG_INFO("Event: NRF_GRTC_EVENT_RTCOMPARE."); - nrfx_grtc_channel_t const * p_channel = &m_cb.channel_data[GRTC_RTCOUNTER_CC_HANDLER_IDX]; - if (p_channel->handler) + if (idx == NRFY_EVENT_TO_INT_BITPOS(NRF_GRTC_EVENT_RTCOMPARE)) { - p_channel->handler((int32_t)GRTC_RTCOUNTER_COMPARE_CHANNEL, - nrfy_grtc_rt_counter_cc_get(NRF_GRTC), - p_channel->p_context); - } - } - - if (active_int_mask & NRF_GRTC_INT_RTCOMPARESYNC_MASK) - { - NRFX_LOG_INFO("Event: NRF_GRTC_EVENT_RTCOMPARESYNC."); - if (m_cb.rtcomparesync_handler) - { - m_cb.rtcomparesync_handler(m_cb.rtcomparesync_context); - } - } + nrfx_grtc_channel_t const * p_channel = + &m_cb.channel_data[GRTC_RTCOUNTER_CC_HANDLER_IDX]; + if (p_channel->handler) + { + p_channel->handler((int32_t)GRTC_RTCOUNTER_COMPARE_CHANNEL, + nrfy_grtc_rt_counter_cc_get(NRF_GRTC), + p_channel->p_context); + } + } #endif // NRF_GRTC_HAS_RTCOUNTER #if NRFY_GRTC_HAS_EXTENDED && NRFY_GRTC_HAS_SYSCOUNTERVALID - /* The SYSCOUNTERVALID bit is automatically cleared when GRTC goes into sleep state and set - * when returning from this state. It can't be cleared inside the ISR procedure because we rely - * on it during SYSCOUNTER value reading procedure. */ - if (nrfy_grtc_event_check(NRF_GRTC, NRF_GRTC_EVENT_SYSCOUNTERVALID) && - nrfy_grtc_int_enable_check(NRF_GRTC, NRF_GRTC_INT_SYSCOUNTERVALID_MASK)) - { - NRFX_LOG_INFO("Event: NRF_GRTC_EVENT_SYSCOUNTERVALID."); - if (m_cb.syscountervalid_handler) + if (idx == NRFY_EVENT_TO_INT_BITPOS(NRF_GRTC_EVENT_SYSCOUNTERVALID)) { - m_cb.syscountervalid_handler(m_cb.syscountervalid_context); + /* The SYSCOUNTERVALID bit is automatically cleared when GRTC goes into sleep state + * and set when returning from this state. It can't be cleared inside the ISR + * procedure because we rely on it during SYSCOUNTER value reading procedure. */ + NRFX_LOG_INFO("Event: NRF_GRTC_EVENT_SYSCOUNTERVALID."); + if (m_cb.syscountervalid_handler) + { + m_cb.syscountervalid_handler(m_cb.syscountervalid_context); + } } - } #endif // NRFY_GRTC_HAS_EXTENDED && NRFY_GRTC_HAS_SYSCOUNTERVALID + } } void nrfx_grtc_irq_handler(void) From 61262d2f5c2b0189ef1c29dcce522989037b4e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Chru=C5=9Bci=C5=84ski?= Date: Mon, 31 Mar 2025 12:39:50 +0200 Subject: [PATCH 3/3] nrfx: drivers: grtc: Add function dedicated for nrf_grtc_timer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add functions that improves performance of nrf_grtc_timer: - Channel allocation with callback. Previously callback is written on every CC val setting which is redundant - Function for setting CCADD register - Function for setting CC register Signed-off-by: Krzysztof Chruściński --- nrfx/drivers/include/nrfx_grtc.h | 60 +++++++++++++++++- nrfx/drivers/src/nrfx_grtc.c | 104 ++++++++++++++++++++++++++++--- 2 files changed, 154 insertions(+), 10 deletions(-) diff --git a/nrfx/drivers/include/nrfx_grtc.h b/nrfx/drivers/include/nrfx_grtc.h index e427f10c..572e3bf2 100644 --- a/nrfx/drivers/include/nrfx_grtc.h +++ b/nrfx/drivers/include/nrfx_grtc.h @@ -169,6 +169,19 @@ nrfx_err_t nrfx_grtc_sleep_configuration_get(nrfx_grtc_sleep_config_t * p_sleep_ */ nrfx_err_t nrfx_grtc_channel_alloc(uint8_t * p_channel); +/** + * @brief Function for setting a callback to a channel. + * + * Function enabled the interrupt for that channel. + * + * @param[in] channel Pointer to the capture/compare channel. + * @param[in] handler User handler called when channel expires. + * @param[in] p_context Context passed to the callback. + */ +void nrfx_grtc_channel_callback_set(uint8_t channel, + nrfx_grtc_cc_handler_t handler, + void * p_context); + /** * @brief Function for freeing the GRTC capture/compare channel. * @@ -296,7 +309,6 @@ void nrfx_grtc_syscountervalid_int_disable(void); */ nrfx_err_t nrfx_grtc_syscounter_start(bool busy_wait, uint8_t * p_main_cc_channel); - /** * @brief Function for performing an action for the GRTC. * @@ -359,6 +371,22 @@ nrfx_err_t nrfx_grtc_syscounter_cc_absolute_set(nrfx_grtc_channel_t * p_chan_dat uint64_t val, bool enable_irq); +/** + * @brief Function for setting the absolute compare value for the SYSCOUNTER. + * + * Function must be called with interrupts locked. If %p safe_setting is true then + * it means that previous CC for that channel did not yet expired and it + * was set to a value earlier than %p val so there is a chance that it will + * expire during setting the new value. In that case compare event may be misinterpreted. + * Slower but safe procedure is used in that case. If %p safe_setting is false then + * function just sets new CC value. + * + * @param[in] channel Channel. + * @param[in] val Absolute value to be set in the compare register. + * @param[in] safe_setting Use safe procedure if true or just set CC to a new value if false. + */ +void nrfx_grtc_syscounter_cc_abs_set(uint8_t channel, uint64_t val, bool safe_setting); + /** * @brief Function for setting the relative compare value for the SYSCOUNTER. * @@ -379,6 +407,22 @@ nrfx_err_t nrfx_grtc_syscounter_cc_relative_set(nrfx_grtc_channel_t * bool enable_irq, nrfx_grtc_cc_relative_reference_t reference); +/** + * @brief Function for setting the relative compare value for the SYSCOUNTER. + * + * Function just sets CCADD value and does not attempt to enable or disable the interrupt. + * It assumes that expected channel configuration is done prior to that call. + * Function assumes that previously used CC value has already expired so new value + * can be safely set without a risk of spurious CC expiration. + * + * @param[in] channel Channel. + * @param[in] val Relative value to be set in the CCADD register. + * @param[in] reference Reference. Current counter value or current CC value. + */ +void nrfx_grtc_syscounter_cc_rel_set(uint8_t channel, + uint32_t val, + nrfx_grtc_cc_relative_reference_t reference); + /** * @brief Function for disabling the SYSCOUNTER compare interrupt. * @@ -539,6 +583,15 @@ NRFX_STATIC_INLINE bool nrfx_grtc_sys_counter_cc_enable_check(uint8_t channel); */ NRFX_STATIC_INLINE bool nrfx_grtc_syscounter_compare_event_check(uint8_t channel); +/** + * @brief Function for retrieving CC value. + * + * @param[in] channel Compare channel. + * + * @return Value read from CC register. + */ +NRFX_STATIC_INLINE uint64_t nrfx_grtc_sys_counter_cc_get(uint8_t channel); + #if NRF_GRTC_HAS_RTCOUNTER || defined(__NRFX_DOXYGEN__) /** * @brief Function for reading the GRTC RTCOUNTER value. @@ -590,6 +643,11 @@ NRFX_STATIC_INLINE bool nrfx_grtc_syscounter_compare_event_check(uint8_t channel return nrfy_grtc_sys_counter_compare_event_check(NRF_GRTC, channel); } +NRFX_STATIC_INLINE uint64_t nrfx_grtc_sys_counter_cc_get(uint8_t channel) +{ + return nrfy_grtc_sys_counter_cc_get(NRF_GRTC, channel); +} + #if NRF_GRTC_HAS_RTCOUNTER NRFX_STATIC_INLINE uint64_t nrfx_grtc_rtcounter_get(void) { diff --git a/nrfx/drivers/src/nrfx_grtc.c b/nrfx/drivers/src/nrfx_grtc.c index aee64eb5..b4fa5ba0 100644 --- a/nrfx/drivers/src/nrfx_grtc.c +++ b/nrfx/drivers/src/nrfx_grtc.c @@ -102,6 +102,9 @@ typedef struct nrfx_atomic_t available_channels; /**< Bitmask of available channels. */ uint32_t used_channels; /**< Bitmask of channels used by the driver. */ nrfx_grtc_channel_t channel_data[NRFX_GRTC_CONFIG_NUM_OF_CC_CHANNELS + 1]; /**< Channel specific data. */ + uint8_t ch_to_data[GRTC_NCC_SIZE]; /**< Mapping of channel indext to channel_data index. */ + uint64_t cc_value[NRFX_GRTC_CONFIG_NUM_OF_CC_CHANNELS]; /**< Last CC value. */ + nrfx_atomic_t read_cc_mask; /**< Indicating if CC value must be passed to the handler. */ #if NRF_GRTC_HAS_RTCOUNTER nrfx_grtc_rtcomparesync_handler_t rtcomparesync_handler; /**< User handler corresponding to rtcomparesync event.*/ void * rtcomparesync_context; /**< User context for rtcomparesync event handler. */ @@ -306,6 +309,18 @@ nrfx_err_t nrfx_grtc_syscounter_get(uint64_t * p_counter) return err_code; } +void nrfx_grtc_channel_callback_set(uint8_t channel, + nrfx_grtc_cc_handler_t handler, + void * p_context) +{ + uint8_t ch_data_idx = get_ch_data_index_for_channel(channel); + + m_cb.channel_data[ch_data_idx].handler = handler; + m_cb.channel_data[ch_data_idx].p_context = p_context; + m_cb.channel_data[ch_data_idx].channel = channel; + nrfy_grtc_int_enable(NRF_GRTC, GRTC_CHANNEL_TO_BITMASK(channel)); +} + nrfx_err_t nrfx_grtc_channel_alloc(uint8_t * p_channel) { NRFX_ASSERT(p_channel); @@ -394,7 +409,10 @@ nrfx_err_t nrfx_grtc_init(uint8_t interrupt_priority) for (uint8_t i = 0; i < NRFX_GRTC_CONFIG_NUM_OF_CC_CHANNELS; i++) { - m_cb.channel_data[i].channel = get_channel_for_ch_data_idx(i); + uint8_t ch = get_channel_for_ch_data_idx(i); + + m_cb.channel_data[i].channel = ch; + m_cb.ch_to_data[ch] = i; } nrfy_grtc_int_init(NRF_GRTC, GRTC_ALL_INT_MASK, interrupt_priority, false); @@ -732,6 +750,29 @@ nrfx_err_t nrfx_grtc_syscounter_cc_disable(uint8_t channel) return err_code; } +void nrfx_grtc_syscounter_cc_abs_set(uint8_t channel, uint64_t val, bool safe_setting) +{ + m_cb.cc_value[m_cb.ch_to_data[channel]] = val; + if (safe_setting) + { + nrfy_grtc_sys_counter_cc_set(NRF_GRTC, channel, val); + if (nrfy_grtc_sys_counter_compare_event_check(NRF_GRTC, channel)) + { + uint64_t now; + + nrfx_grtc_syscounter_get(&now); + if (val > now) + { + nrfy_grtc_sys_counter_compare_event_clear(NRF_GRTC, channel); + } + } + } + else + { + nrfy_grtc_sys_counter_cc_set(NRF_GRTC, channel, val); + } +} + nrfx_err_t nrfx_grtc_syscounter_cc_absolute_set(nrfx_grtc_channel_t * p_chan_data, uint64_t val, bool enable_irq) @@ -755,6 +796,7 @@ nrfx_err_t nrfx_grtc_syscounter_cc_absolute_set(nrfx_grtc_channel_t * p_chan_dat if (enable_irq) { + NRFX_ATOMIC_FETCH_OR(&m_cb.read_cc_mask, NRFX_BIT(p_chan_data->channel)); nrfy_grtc_int_enable(NRF_GRTC, GRTC_CHANNEL_TO_BITMASK(p_chan_data->channel)); } @@ -764,6 +806,17 @@ nrfx_err_t nrfx_grtc_syscounter_cc_absolute_set(nrfx_grtc_channel_t * p_chan_dat return err_code; } +void nrfx_grtc_syscounter_cc_rel_set(uint8_t channel, + uint32_t val, + nrfx_grtc_cc_relative_reference_t reference) +{ + m_cb.cc_value[m_cb.ch_to_data[channel]] += val; + nrfy_grtc_sys_counter_cc_add_set(NRF_GRTC, + channel, + val, + (nrf_grtc_cc_add_reference_t)reference); +} + nrfx_err_t nrfx_grtc_syscounter_cc_relative_set(nrfx_grtc_channel_t * p_chan_data, uint32_t val, bool enable_irq, @@ -793,6 +846,7 @@ nrfx_err_t nrfx_grtc_syscounter_cc_relative_set(nrfx_grtc_channel_t * if (enable_irq) { + NRFX_ATOMIC_FETCH_OR(&m_cb.read_cc_mask, NRFX_BIT(p_chan_data->channel)); nrfy_grtc_int_enable(NRF_GRTC, GRTC_CHANNEL_TO_BITMASK(p_chan_data->channel)); } @@ -905,23 +959,53 @@ static void grtc_irq_handler(void) uint32_t intpend = nrf_grtc_int_pending_get(NRF_GRTC); while (intpend) { - uint32_t idx = 31 - NRFX_CLZ(intpend); + uint8_t idx = (uint8_t)(31 - NRFX_CLZ(intpend)); intpend &= ~NRFX_BIT(idx); - nrf_grtc_event_clear(NRF_GRTC, NRFY_INT_BITPOS_TO_EVENT(idx)); - if (!NRFX_IS_ENABLED(GRTC_EXT) || idx < 16) + if (!NRFX_IS_ENABLED(GRTC_EXT) || idx < GRTC_NCC_SIZE) { - for (uint32_t i = 0; i < NRFX_GRTC_CONFIG_NUM_OF_CC_CHANNELS; i++) + uint32_t i = m_cb.ch_to_data[idx]; + + NRFX_ASSERT(m_cb.channel_data[i].channel == idx); + + if (m_cb.channel_data[i].handler) { - if ((m_cb.channel_data[i].channel == idx) && m_cb.channel_data[i].handler) + uint64_t cc_value; + + if (NRFX_ATOMIC_FETCH_AND(&m_cb.read_cc_mask, ~NRFX_BIT(idx)) & NRFX_BIT(idx)) { - m_cb.channel_data[i].handler(idx, m_cb.channel_data[i].p_context); - break; + /* Read CC value only if channel was set using legacy functions. It is done + * for API backward compatibility. Reading 64 bit value from GRTC is costly + * and it is avoided if possible. + */ + cc_value = nrfy_grtc_sys_counter_cc_get(NRF_GRTC, idx); } + else + { + /* If CC was set using optimized API then CC is stored in RAM (much faster + * access). + */ + cc_value = m_cb.cc_value[i]; + } + + /* Check event again (initially checked via INTPEND). It is possible that + * CC is reconfigured from higher priority context. In that case event + * might be cleared. + */ + if (!nrf_grtc_event_check(NRF_GRTC, NRFY_INT_BITPOS_TO_EVENT(idx))) + { + break; + } + + nrf_grtc_event_clear(NRF_GRTC, NRFY_INT_BITPOS_TO_EVENT(idx)); + + m_cb.channel_data[i].handler(idx, cc_value, m_cb.channel_data[i].p_context); + break; } + /* Return early as this is the most likely scenario (single CC expiring). */ - if (intpend == 0) + if (NRFX_IS_ENABLED(GRTC_EXT) && (intpend == 0)) { break; } @@ -929,6 +1013,7 @@ static void grtc_irq_handler(void) #if NRF_GRTC_HAS_RTCOUNTER if (idx == NRFY_EVENT_TO_INT_BITPOS(NRF_GRTC_EVENT_RTCOMPARE)) { + nrf_grtc_event_clear(NRF_GRTC, NRFY_INT_BITPOS_TO_EVENT(idx)); nrfx_grtc_channel_t const * p_channel = &m_cb.channel_data[GRTC_RTCOUNTER_CC_HANDLER_IDX]; if (p_channel->handler) @@ -946,6 +1031,7 @@ static void grtc_irq_handler(void) * and set when returning from this state. It can't be cleared inside the ISR * procedure because we rely on it during SYSCOUNTER value reading procedure. */ NRFX_LOG_INFO("Event: NRF_GRTC_EVENT_SYSCOUNTERVALID."); + nrf_grtc_event_clear(NRF_GRTC, NRFY_INT_BITPOS_TO_EVENT(idx)); if (m_cb.syscountervalid_handler) { m_cb.syscountervalid_handler(m_cb.syscountervalid_context);