espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
12.91k stars 7.08k forks source link

PHY Calibration Efficiency Regression (IDFGH-12197) #13251

Open boarchuz opened 4 months ago

boarchuz commented 4 months ago

Answers checklist.

IDF version.

v5.1.1-260-g9331e240fc

Espressif SoC revision.

ESP32

Operating System used.

Linux

How did you build your project?

VS Code IDE

If you are using Windows, please specify command line type.

None

Development Kit.

Custom Board

Power Supply used.

USB

What is the expected behavior?

Phy calibration should be skipped if the calibration mode is PHY_RF_CAL_NONE.

What is the actual behavior?

As of 9331e240fcae2a3ae37b4dbb323f78c8220caffb, a ~2ms high-power period has been introduced in phy calibration even when using PHY_RF_CAL_NONE with valid calibration data.

The following are captures of the call to register_chipv7_phy(PHY_RF_CAL_NONE) using a Joulescope to measure power consumption and time. The main thing to note is the very high power peaks for ~2ms in the middle of first image.

BAD (9331e240fcae2a3ae37b4dbb323f78c8220caffb) 5.8ms, average 77mA, max 458mA: phyissue_bad

GOOD (98f4ce20119ae9454ddc88f8b2c4d2267cc49e91, ie. previous commit) 3.6ms, average 22mA, max 117mA: phyissue_good

Steps to reproduce.

main/main.c:

#include <stdio.h>
#include <string.h>
#include <stdbool.h>

#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "esp_err.h"
#include "esp_log.h"
#include "esp_sleep.h"
#include "nvs_flash.h"
#include "esp_wifi.h"
#include "esp_wifi_default.h"
#include "esp_phy_init.h"
#include "driver/gpio.h"

#include "sdkconfig.h"

static const char *TAG = "phy_issue";

#define PHY_CAL_TRACE_PIN 26

// Wrap register_chipv7_phy in order to output trace signal
int __wrap_register_chipv7_phy(const esp_phy_init_data_t* init_data, esp_phy_calibration_data_t *cal_data, esp_phy_calibration_mode_t cal_mode)
{
    int __real_register_chipv7_phy(const esp_phy_init_data_t* init_data, esp_phy_calibration_data_t *cal_data, esp_phy_calibration_mode_t cal_mode);

    #ifdef PHY_CAL_TRACE_PIN
        for(int i = 0; i < 3; ++i)
        {
            gpio_set_level(PHY_CAL_TRACE_PIN, 0);
            esp_rom_delay_us(10);
            gpio_set_level(PHY_CAL_TRACE_PIN, 1);
            esp_rom_delay_us(10);
        }
    #endif
    int ret = __real_register_chipv7_phy(init_data, cal_data, cal_mode);
    #ifdef PHY_CAL_TRACE_PIN
        for(int i = 0; i < 3; ++i)
        {
            gpio_set_level(PHY_CAL_TRACE_PIN, 0);
            esp_rom_delay_us(10);
            gpio_set_level(PHY_CAL_TRACE_PIN, 1);
            esp_rom_delay_us(10);
        }
    #endif
    ESP_LOGI(TAG, "Cal mode: %d, err: %d", cal_mode, ret);
    return ret;
}

void app_main(void)
{
    // Initialise Trace Pin
    #ifdef PHY_CAL_TRACE_PIN
        gpio_set_level(PHY_CAL_TRACE_PIN, 1);
        gpio_config_t config = {
            .pin_bit_mask = 1ULL << PHY_CAL_TRACE_PIN,
            .mode = GPIO_MODE_OUTPUT,
            .pull_up_en = 1,
        };
        ESP_ERROR_CHECK(gpio_config(&config));
    #endif

    // Start WiFi (will trigger phy calibration)
    nvs_flash_init();
    esp_netif_init();
    esp_event_loop_create_default();
    esp_netif_create_default_wifi_sta();
    esp_wifi_init(&(wifi_init_config_t)WIFI_INIT_CONFIG_DEFAULT());
    esp_wifi_set_mode(WIFI_MODE_STA);
    esp_wifi_start();

    vTaskDelay(1000 / portTICK_PERIOD_MS);

    // Repeat
    esp_wifi_stop();
    vTaskDelay(100 / portTICK_PERIOD_MS);
    esp_deep_sleep(1 * 1000 * 1000);
}

main/CMakeLists.txt:

idf_component_register(
    SRCS "main.c"
)
idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=register_chipv7_phy" APPEND)

Debug Logs.

No response

More Information.

No response

mhdong commented 4 months ago

Hi @boarchuz Please use the following esp32 esp_phy lib test. We will fix this issue as soon as possible. libphy_esp32_20240226_ea290180.zip

boarchuz commented 4 months ago

Hi @mhdong, That seems to fix the specific issue, though I do want to note that it is still ~500us slower than previous. I'm not sure if this is an unfortunate symptom of the increasing size and complexity of ESP-IDF and binary blobs, or if this is an unexpected bug?

phyissue_end

Note, in particular the unusual period towards the end of calibration marked with blue flags.

Actually these appear to also be in the "BAD" image above, yet are much less apparent due to the Y scale, so this was introduced in the same commit. Is this intended?

mhdong commented 4 months ago

Hi @boarchuz The fluctuation marked with blue flag is intended. Comparing with the previous version, the additional time is for better RF performance.

AxelLin commented 1 month ago

Hi @boarchuz Please use the following esp32 esp_phy lib test. We will fix this issue as soon as possible.

@mhdong Is this fixed or not yet?