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

pinMode is much slower in 2.0.3 (and 2.0.4) #7049

Closed
1 task done
jenscski opened this issue Jul 28, 2022 · 6 comments
Closed
1 task done

pinMode is much slower in 2.0.3 (and 2.0.4) #7049

jenscski opened this issue Jul 28, 2022 · 6 comments
Assignees
Labels
Area: Performance Issue related to performance problems and improvements Area: Peripherals API Relates to peripheral's APIs. Resolution: Wontfix Arduino ESP32 team will not fix the issue
Milestone

Comments

@jenscski
Copy link

Board

All boards

Device Description

Tested with featheresp32

Hardware Configuration

DS18B20 connected to A0

Version

v2.0.3

IDE Name

not applicable

Operating System

not applicable

Flash frequency

not applicable

PSRAM enabled

no

Upload speed

not applicable

Description

In version 2.0.3 the GPIO handing was rewritten using IDF instead of the old implementation.

This caused the time pinMode takes to finish from ~4us to ~16us.

In ESPHome the one wire implementation uses the normal Arduino calls to manipulate GPIO, and this change is probably the cause of One Wire not working in ESPHome when using Arduino 2.0.3.

esphome/issues#3401

Is it possible to speed up pinMode?

After studying the ESPHome and Arduino code, I see that when ESPHome is compiled using ESP-IDF, it uses a different approach than Arduino for setting pin mode.

ESPHome https://github.com/esphome/esphome/blob/ed26c57d990c77cf9eed9bd54da2681f8f7881c9/esphome/components/esp32/gpio_idf.cpp#L83-L106

Arduino

extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode)
{
#ifdef RGB_BUILTIN
if (pin == RGB_BUILTIN){
__pinMode(RGB_BUILTIN-SOC_GPIO_PIN_COUNT, mode);
return;
}
#endif
if (!GPIO_IS_VALID_GPIO(pin)) {
log_e("Invalid pin selected");
return;
}
gpio_hal_context_t gpiohal;
gpiohal.dev = GPIO_LL_GET_HW(GPIO_PORT_0);
gpio_config_t conf = {
.pin_bit_mask = (1ULL<<pin), /*!< GPIO pin: set with bit mask, each bit maps to a GPIO */
.mode = GPIO_MODE_DISABLE, /*!< GPIO mode: set input/output mode */
.pull_up_en = GPIO_PULLUP_DISABLE, /*!< GPIO pull-up */
.pull_down_en = GPIO_PULLDOWN_DISABLE, /*!< GPIO pull-down */
.intr_type = gpiohal.dev->pin[pin].int_type /*!< GPIO interrupt type - previously set */
};
if (mode < 0x20) {//io
conf.mode = mode & (INPUT | OUTPUT);
if (mode & OPEN_DRAIN) {
conf.mode |= GPIO_MODE_DEF_OD;
}
if (mode & PULLUP) {
conf.pull_up_en = GPIO_PULLUP_ENABLE;
}
if (mode & PULLDOWN) {
conf.pull_down_en = GPIO_PULLDOWN_ENABLE;
}
}
if(gpio_config(&conf) != ESP_OK)
{
log_e("GPIO config failed");
return;
}
}

Sketch

This is the sketch I ran using 2.0.2 and 2.0.3. pinMode was the only one with different timing


#include <Arduino.h>

void test()
{
  auto d1 = micros();
  pinMode(A0, OUTPUT);
  auto d2 = micros();
  digitalWrite(A0, LOW);
  auto d3 = micros();
  pinMode(A0, INPUT_PULLUP);
  auto d4 = micros();
  digitalRead(A0);
  auto d5 = micros();

  Serial.println(d2 - d1);
  Serial.println(d3 - d2);
  Serial.println(d4 - d3);
  Serial.println(d5 - d4);
  Serial.println();
}

void setup()
{
  Serial.begin(9600);
  Serial.println();
  Serial.println("Hello");

  test();
  test();
  test();
  test();
}

void loop()
{
  // put your main code here, to run repeatedly:
}

Debug Message

not applicable

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@jenscski jenscski added the Status: Awaiting triage Issue is waiting for triage label Jul 28, 2022
@Jason2866
Copy link
Collaborator

Jason2866 commented Jul 28, 2022

Speed improvement is always nice to have. Dropping the manipulating of registers and using IDF is imho a good decision. Otoh there is a bit more overhead for some functions, which can decrease speed. The better solution imho is to adopt the code (to the new general conditions) to be (more) timing tolerant, as to hope that a speed increase will happen
btw this OneWire lib does work: https://github.com/PaulStoffregen/OneWire

@TD-er
Copy link
Contributor

TD-er commented Jul 28, 2022

Ah this is a plausible explanation why the Dallas plugin and several others like DHTxx no longer seem to work on ESP32 in ESPEasy.

@TD-er
Copy link
Contributor

TD-er commented Jul 28, 2022

btw this OneWire lib does work: https://github.com/PaulStoffregen/OneWire

That looks like an easy approach to speed up switching the mode by simply keeping track of the register and bitmask for a given pin.

I will at least implement this for my One Wire implementation and probably also for other timing critical plugins like those DHTxx units.

TD-er added a commit to TD-er/ESPEasy that referenced this issue Jul 28, 2022
Apparently the `pinMode` call on ESP SDK 2.0.3 and 2.0.4 now takes 16 usec to complete, compared to 4 msec before.

The [OneWire library maintained by PaulStoffregen](https://github.com/PaulStoffregen/OneWire/blob/master/OneWire.cpp) does perform direct GPIO handling on the registers instead of searching for the pins.
This is apparently working as reported [here by @Jason2866](espressif/arduino-esp32#7049 (comment))

So I replaced all time critical calls for the Dallas plugins with the macros from Paul's library.
@TD-er
Copy link
Contributor

TD-er commented Jul 31, 2022

OK, setting pinMode to INPUT_PULLUP is even worse.
I was working on the implementation of the DHT22 support for ESPEasy to improve timing stability and apparently setting the pin mode to input + pull-up takes a whoppin' 228 usec.

image

The white line is the pin I set to INPUT_PULLUP, which is then pulled down by the DHT22 sensor.
The brown line is an extra pin (GPIO-27) I toggle using direct GPIO access:

    DIRECT_WRITE_HIGH(reg, mask_27);  // GPIO-27
    pinMode(DHT_pin, INPUT_PULLUP);
    DIRECT_WRITE_LOW(reg, mask_27);  // GPIO-27

This is it with direct access setting the pin mode to INPUT (thus not using the pull-up as I don't know how to do it via direct access)
image

    DIRECT_WRITE_HIGH(reg, mask_27);
    DIRECT_MODE_INPUT(reg, mask);
//    pinMode(DHT_pin, INPUT_PULLUP);
    DIRECT_WRITE_LOW(reg, mask_27);

This takes 4 usec to set the pin mode.

@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented Jan 17, 2023

@TD-er, GPIO has been changed to use the IDF API, rather than directly writing to registers in order to have support for new chips. I'm closing this and adding label "won't fix", as there is nothing to do about this issue.

@P-R-O-C-H-Y P-R-O-C-H-Y closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2023
@P-R-O-C-H-Y P-R-O-C-H-Y added Resolution: Wontfix Arduino ESP32 team will not fix the issue Area: Performance Issue related to performance problems and improvements Area: Peripherals API Relates to peripheral's APIs. and removed Status: Awaiting triage Issue is waiting for triage labels Jan 17, 2023
@TD-er
Copy link
Contributor

TD-er commented Jan 17, 2023

Not 100% sure it is something that can't be done.
In my own implementation of timing critical stuff, I now use the direct access.
But IMHO it is not the best approach to have an SDK for which something like an additional library (or at least extra code) is needed to handle "direct GPIO access".
I'm OK with the idea to not change the "Arduino-like" calls to GPIO pins.
But at least there should be something in the SDK to have a more or less uniform way to use direct-IO access.
Otherwise there will be a lot of different implementations for this, which may all fail in a different way on the next major update of the SDK.

Also I have no idea how good or bad "my" implementation is and what side-effects this may have on the IDF implementation of GPIO-handling.

TL;DR I don't think just dismissing it "as is -> won't fix" is the best solution here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Performance Issue related to performance problems and improvements Area: Peripherals API Relates to peripheral's APIs. Resolution: Wontfix Arduino ESP32 team will not fix the issue
Projects
Development

No branches or pull requests

5 participants