Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fast IO access #240

Draft
wants to merge 46 commits into
base: main
Choose a base branch
from
Draft

Fast IO access #240

wants to merge 46 commits into from

Conversation

elral
Copy link
Collaborator

@elral elral commented Mar 30, 2023

Description of changes

For each device with multiple accesses to I/O's the arduino functions digitalread() and digitalWrite()is replaced by direct port access to reduce the time each changed device needs.
This will allow to increase the number of multiplexers while keeping the functionality of encoders and will give the possibility to implement encoders connected to multiplexers.

As this change will exceed the flash for the ProMicro, it will be only implemented for:

  • Mega
  • Uno/Nano
    per compile flag and could be implemented for the Pico (not changed so far).

More description is available in issue #239

Fixes #239

@github-actions
Copy link

Firmware for this pull request:
Firmware.zip

@GioCC
Copy link
Contributor

GioCC commented Mar 31, 2023

A few first-look comments:

  • Storing two bytes for every "fast" pin (port + mask) wastes a lot of RAM.
    Having these values ready probably only makes sense for driver pins (shift registers, multiplexers, LED drivers), whose number is limited (and could be even more limited if they were shared) and whose usage is much more intensive. It's not as important e.g. for buttons.
    If a better performance than the standard digitalRead is still desired, you could have a second overload of the "fast" versions, with the simple pin number as argument, to be used in "regular" cases.
    This overload would incorporate the digitalPinToBitMask() and portOutputRegister(digitalPinToPort()) calls; these are simple macro lookups and therefore still remarkably faster than the standard functions.
    It could be evaluated if these "less optimized" functions have sufficient performance to be used also for driver pins.

  • I see that in some cases, both the pin number and the pin port+mask are stored; I wonder if it's possible to drop pin number (not examined the code to this extent yet).

  • In digitalWriteFast(), the read->update->write operation is supposed to be atomic, therefore it's necessary to add the "Save SREG / cli" sequence as used in the default function.

@elral
Copy link
Collaborator Author

elral commented Mar 31, 2023

A few first-look comments:

  • Storing two bytes for every "fast" pin (port + mask) wastes a lot of RAM.
    Having these values ready probably only makes sense for driver pins (shift registers, multiplexers, LED drivers), whose number is limited (and could be even more limited if they were shared) and whose usage is much more intensive. It's not as important e.g. for buttons.
    If a better performance than the standard digitalRead is still desired, you could have a second overload of the "fast" versions, with the simple pin number as argument, to be used in "regular" cases.
    This overload would incorporate the digitalPinToBitMask() and portOutputRegister(digitalPinToPort()) calls; these are simple macro lookups and therefore still remarkably faster than the standard functions.
    It could be evaluated if these "less optimized" functions have sufficient performance to be used also for driver pins.

Give me some time to think about (and understand ;) ) this

  • I see that in some cases, both the pin number and the pin port+mask are stored; I wonder if it's possible to drop pin number (not examined the code to this extent yet).

Yeah, I thought about this a longer time. Idea's are welcome.
But this opens another discussion. The pin numbes are required in some detach() functions, but not in all. The newer ones switches the pins back to input where the "older" ones leave the state as they are. A clean solution is the first appoach (switching back the pin to input), but is it really required? If so, shouldn't we change it for the detach()functions where it is not done? But maybe better to discuss this in a separate topic.

  • In digitalWriteFast(), the read->update->write operation is supposed to be atomic, therefore it's necessary to add the "Save SREG / cli" sequence as used in the default function.

Good point!

Thanks and regards

Ralf

@GioCC
Copy link
Contributor

GioCC commented Mar 31, 2023

Thanks to you for trying to find even more improvements!

OK, hoping to spare you some time by trying to be clearer:
let's say we add another function in MFFastIO.h...

inline uint8_t digitalReadFast(uint8_t pinNr)
{
    return (*portOutputRegister(digitalPinToPort(pinNr) & digitalPinToBitMask(pinNr)) ? HIGH : LOW;
}

this way, e.g. Buttons would continue to use pin numbers without the need to store pin port and pin mask.

About the detach() functions: even there, I would imagine that a "fast" version of the pinMode() could be introduced to reset the pin to input. I haven't looked at the code yet though.

Switching unused pins back to input is definitely a thing to recommend though.

@github-actions
Copy link

Firmware for this pull request:
Firmware.zip

@github-actions
Copy link

Firmware for this pull request:
Firmware.zip

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

github-actions bot commented Jan 5, 2024

Firmware for this pull request:
Firmware.zip

Copy link

github-actions bot commented Jan 8, 2024

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

Firmware for this pull request:
Firmware.zip

Copy link

github-actions bot commented May 5, 2024

Firmware for this pull request:
Firmware.zip

Copy link

github-actions bot commented May 6, 2024

Firmware for this pull request:
Firmware.zip

Copy link

Board and firmware folder for this pull request:
Mobiflight-Connector.zip

Copy link

Board and firmware folder for this pull request:
Mobiflight-Connector.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accelerate reading/writing to I/O's
2 participants