KevinOConnor / can2040

Software CAN bus implementation for rp2040 micro-controllers
GNU General Public License v3.0
636 stars 63 forks source link

ISR in RAM ? #18

Closed HondaRulez closed 1 year ago

HondaRulez commented 1 year ago

Hi,

Any plan to moving the ISR into the RAM to minimise the latency due the cache misses ?

KevinOConnor commented 1 year ago

Moving the code into ram is heavily dependent on the build environment of the project that is using can2040. It's generally possible to implement it by modifying the build's linker scripts - code changes to can2040 shouldn't be necessary.

-Kevin

kentindell commented 1 year ago

Moving the code into ram is heavily dependent on the build environment of the project that is using can2040. It's generally possible to implement it by modifying the build's linker scripts - code changes to can2040 shouldn't be necessary.

The functions that should be in RAM need to be marked as time critical to put them into the necessary sector defined by the linker. I use a decorator for the function of TIME_CRITICAL like so:

And then the header file in the build process defines it as:

#define     TIME_CRITICAL                   __attribute__((noinline, long_call, section(".time_critical")))

Used like this:

TIME_CRITICAL bool send_bits(ctr_t bit_end, ctr_t sample_point, struct canhack *canhack_p, uint8_t tx_index, canhack_frame_t *frame)

The linker file then defines a section time_critical like this:

    .data : {
        __data_start__ = .;
        *(vtable)

        *(.time_critical*)

This is how the MicroPython build for the RP2040 works. See:

https://github.com/micropython/micropython/blob/master/ports/rp2/memmap_mp.ld

for the linker file.

The vector table needs to be in RAM too, otherwise that gets hit with huge delays. And ideally, all the mutexes that lock out interrupts, otherwise the interrupts get delayed by cache hits due to priority inversion. The whole XIP flash thing is a minefield, with huge delays turning up in all kinds of places:

https://kentindell.github.io/2021/03/05/pico-priority-inversion/

KevinOConnor commented 1 year ago
    *(.time_critical*)

FYI, an alternative that does not require modifications to the can2040.c code is to instead add can2040.o(.text*) to the linker script. Another alternative is to enable gcc -flto optimizations and tag the irq handler in the calling code as a "ram function" (such that gcc will inline the can2040_pio_irq_handler() into the calling function and then place that entire function into ram). Another alternative is to change the build to place the entire application in ram (if it is small enough).

Cheers, -Kevin

kentindell commented 1 year ago

My personal preference, if possible, is to put the whole application into RAM and disable XIP flash. It's way too easy to unexpectedly hit a cache miss and lock the CPU for an eternity. Obviously that's not possible for something as large as MicroPython but many many applications will fit in the RAM.

github-actions[bot] commented 1 year ago

Hello,

It looks like there hasn't been any recent updates on this github ticket. We prefer to only list tickets as "open" if they are actively being worked on. Feel free to provide an update on this ticket. Otherwise the ticket will be automatically closed in a few days.

Best regards,

~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.