micros() overflow fix

[jpmeijers]

The micros() function uses the cycle count register of the CPU. This function divides the cycle count by the number of cycles in a microsecond and returns that value. The problem is when the cycle count overflows at the 32bit maximum value, the microsecond value will also overflow. Many applications expect the microsecond value not to overflow before it reaches 2^32.

This function now keeps track of cycle count overflows, and take them into account when calculating the microseconds.

[me-no-dev]

that is true, given that you call micros often enough :) else it will again go out of sync.

[igrr]

Why not call gettimeofday instead? This will fail if the CPU frequency is not constant.

[me-no-dev]

@igrr I remember having issues with gettimeofday a while back and resorted to the current code. Will look into it again. Things change on both fronts as we go :) I wonder how many unnecessary workarounds we will find with time.

[me-no-dev]

@igrr testedgettimeofday and here is my issue with it: it takes 11us to query VS 0.1 us for current code.
So in this case no delay under 11us or in steps lower than 11us is possible.

[igrr]

Ok, I see. Will add a cheaper "get number of microseconds since boot" function...
The proposed fix seems to be better than no fix for now.

[me-no-dev]

How about we calculate overflow count in a thread and keep the time accurate. The reading of ccounter and incrementing overflow is cheap.

[me-no-dev]

OK the following code costs 0.2us. If no one has arguments, I'll commit it.

uint32_t IRAM_ATTR micros()
{
    static uint32_t lccount = 0;
    uint32_t ccount;
    static uint32_t overflow = 0;
    __asm__ __volatile__ ( "rsr     %0, ccount" : "=a" (ccount) );
    if(ccount < lccount){
        overflow += 4294967295 / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ;
    }
    lccount = ccount;
    return overflow + (ccount / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ);
}

[igrr]

You may possibly want to replace that integer constant with a UINT32_MAX, but aside from that it's fine (with the above mentioned caveat about calling micros often enough). If you add a call to micros to the idle hook, there should be no issues at all.
Obviously, you need to require micros to be called from one task only, otherwise you risk having a race condition when two tasks try to increment overflow counter at the same time. If micros is not used in ISRs, that's probably not an issue.

[me-no-dev]

Yup :) gave it a go in another task and... havoc :D looking into better solution.

[me-no-dev]

maybe something like this:

portMUX_TYPE microsMux = portMUX_INITIALIZER_UNLOCKED;

uint32_t IRAM_ATTR micros()
{
    static uint32_t lccount = 0;
    static uint32_t overflow = 0;
    uint32_t ccount;
    __asm__ __volatile__ ( "rsr     %0, ccount" : "=a" (ccount) );
    if(ccount < lccount){
        if(xPortInIsrContext()){
            portENTER_CRITICAL_ISR(&microsMux);
            overflow += UINT32_MAX / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ;
            portEXIT_CRITICAL_ISR(&microsMux);
        } else {
            portENTER_CRITICAL(&microsMux);
            overflow += UINT32_MAX / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ;
            portEXIT_CRITICAL(&microsMux);
        }
    }
    lccount = ccount;
    return overflow + (ccount / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ);
}

[me-no-dev]

maybe makes the IRAM pointless?

[igrr]

FYI you can use portENTER_CRITICAL_ISR from task context as well. This is not the case for other FreeRTOS ports, but for the ESP32 we had to make it so.

There is still a race condition in the code above. Consider the following case:

1. task calls micros, reads ccount, ccount is (UINT32_MAX - small number)
2. ISR happens, preempts the task. ISR does some stuff, calls micros, ccount is now (0 + small number)
3. overflow is incremented in ISR context, lccount is set to (small number).
4. ISR exits and the task resumes.
5. Task proceeds to execute lccount = ccount, setting lccount to the obtained value of ccount (UINT32_MAX - small number)
6. microseconds value returned to the task will be off by ~UINT32_MAX / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ
7. Next call to micros sees that ccount is (slightly bigger, but still small number), and lccount is (UINT32_MAX - small number), and proceeds to increment overflow again, making the clock skip ahead by about UINT32_MAX / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ - something.

How bad is grabbing portENTER_CRITICAL_ISR each time? We have a fix in pipeline which reduces the penalty for taking a spinlock (see some numbers in MR 497), so i would probably suggest to go this way.

[me-no-dev]

@igrr yeah I though of that and was looking for a way to maybe do it without CRITICAL every time. The following code "costs" 800-1000ns depending if overflow happened or not. Quite more than the 200ns that we had, but might be some sort of a compromise. Keep in mind, this number will change as the CPU is clocked down.

portMUX_TYPE microsMux = portMUX_INITIALIZER_UNLOCKED;

uint32_t IRAM_ATTR micros()
{
    static uint32_t lccount = 0;
    static uint32_t overflow = 0;
    uint32_t ccount;
    portENTER_CRITICAL_ISR(&microsMux);
    __asm__ __volatile__ ( "rsr     %0, ccount" : "=a" (ccount) );
    if(ccount < lccount){
        overflow += UINT32_MAX / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ;
    }
    lccount = ccount;
    portEXIT_CRITICAL_ISR(&microsMux);
    return overflow + (ccount / CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ);
}

[me-no-dev]

Implemented :)