esp32-s3 - Calling LovyanGFX breaks the SPI bus for other devices

LouDou commented 5 months ago

Carefully written Issues are more likely to be given priority. 丁寧に記述された報告は優先して対応される可能性が高くなります。

Environment ( 実行環境 )

MCU or Board name: esp32-s3-devkitc-1
Panel Driver IC: Panel_ST7735S
Bus type: SPI
LovyanGFX version: 1.1.9
FrameWork version: ESP-IDF v5.1.1
Build Environment: PlatformIO
Operating System: Linux

Problem Description ( 問題の内容 )

I am sharing the SPI bus between one ST7735s and two AD5238 DAC devices. I have both the display and DACs working independently. However, as soon as I send anything to the display, the DAC stops working. The DAC is configured using the esp-idf spi_bus_add_device API and also sets up the bus using spi_bus_initialize.

Note that the display uses SPI mode 0 and the DACs require SPI mode 2.

Expected Behavior ( 期待される動作 )

Both the DAC and display can be used at the same time on the same SPI bus.

Actual Behavior ( 実際の動作 )

DAC stops working after the display has been updated.

Steps to reproduce ( 再現のための前提条件 )

Using DAC on its own works perfectly
Using the display on its own works perfectly
Usng the DAC and then the display, both work
Using the display and then the DAC - the display is updated, but the DAC stops responding to any further commands

Code to reproduce this issue ( 再現させるためのコード )

DAC interface:


namespace psc::dac
{
    // configure CS GPIOs - control; output only
    constexpr int cs_gpios[] = {
        DAC_1_CS,
        DAC_2_CS,
    };

    spi_device_handle_t dacs[2];

    spi_transaction_t t{
        .flags = SPI_TRANS_USE_TXDATA,
        .length = 2 * 8,
    };

    inline void writeDac(const size_t idx, const uint16_t data)
    {
        t.tx_data[0] = data >> 8;
        t.tx_data[1] = data & 0xFF;
        /* auto ret = */ spi_device_polling_transmit(dacs[idx], &t);
        // /* auto ret = */ spi_device_transmit(dacs[idx], &t);
        // /* auto ret = */ spi_device_queue_trans(dacs[idx], &t, portMAX_DELAY);
        // ESP_LOGI(TAG, "write %d 0x%02x : 0x%02x : 0x%02x", cs, t.tx_data[0], t.tx_data[1], ret);
    }

    inline void writeDacOutput(const size_t idx, const uint8_t ch, const uint16_t val)
    {
        // Page 16 "DAC Write" / Figure 34: Shift register contents
        // [RW, A2, A1, A0, d12...d0]
        // RW; 0=Write DAC value
        writeDac(idx, (ch << 12) | (val & 0xFFF));
    }

    void setup()
    {
        esp_err_t ret;

        // -----

        // Initialize the SPI bus
        spi_bus_config_t buscfg = {
            .mosi_io_num = SPI_MOSI,
            .miso_io_num = SPI_MISO,
            .sclk_io_num = SPI_SCLK,
        };
        ret = spi_bus_initialize(SPI2_HOST, &buscfg, SPI_DMA_CH_AUTO);
        ESP_ERROR_CHECK(ret);

        // -----

        gpio_config_t io_cs_conf = {
            .pin_bit_mask = 0,
            .mode = GPIO_MODE_OUTPUT,
            .pull_up_en = GPIO_PULLUP_DISABLE,
            .pull_down_en = GPIO_PULLDOWN_ENABLE,
            .intr_type = GPIO_INTR_DISABLE,
        };
        for (int i = 0; i < sizeof(cs_gpios) / sizeof(cs_gpios[0]); i++)
        {
            io_cs_conf.pin_bit_mask |= 1ULL << cs_gpios[i];
        }
        gpio_config(&io_cs_conf);

        // -----

        // Attach the DACs to the SPI bus
        spi_device_interface_config_t devcfg0 = {
            .mode = 2,
            .clock_speed_hz = SPI_MASTER_FREQ_8M,
            .spics_io_num = DAC_1_CS,
            .queue_size = 1,
        };
        ret = spi_bus_add_device(SPI2_HOST, &devcfg0, &dacs[0]);
        ESP_ERROR_CHECK(ret);

        spi_device_interface_config_t devcfg1 = {
            .mode = 2,
            .clock_speed_hz = SPI_MASTER_FREQ_8M,
            .spics_io_num = DAC_2_CS,
            .queue_size = 1,
        };
        ret = spi_bus_add_device(SPI2_HOST, &devcfg1, &dacs[1]);
        ESP_ERROR_CHECK(ret);

        // -----

        // initialise CS
        for (auto pin : cs_gpios)
        {
            gpio_set_level((gpio_num_t)pin, 1); // CS /SYNC is inverted
        }
        // configure DACs
        for (size_t idx : {0, 1})
        {
            // Page 18: Gain, BUF and Vdd config
            const uint16_t cfg = (1 << 15) | 0b00'00'11;
            writeDac(idx, cfg);

            // Table 8: /LDAC
            // /LDAC low mode; continuous update using SYNC;
            // /LDAC must be tied high
            const uint16_t lcadmode = (1 << 15) | (1 << 13) | 0x00;
            writeDac(idx, lcadmode);
        }
    }
}

SPI and display interface


namespace psc::io
{
    class PSCSPI : public lgfx::Bus_SPI
    {
    public:
        PSCSPI()
            : lgfx::Bus_SPI()
        {
            auto cfg = config();

            cfg.spi_host = SPI2_HOST;
            cfg.spi_mode = 0;

            cfg.freq_write = 40'000'000; // for TFT
            cfg.freq_read = 2'500'000;
            cfg.spi_3wire = false;
            cfg.use_lock = true;
            cfg.dma_channel = SPI_DMA_CH_AUTO;

            cfg.pin_sclk = SPI_SCLK;
            cfg.pin_mosi = SPI_MOSI;
            cfg.pin_miso = SPI_MISO;
            cfg.pin_dc = TFT_DC;

            config(cfg);
            init();
        }
    };

    using SPI_sptr = std::shared_ptr<PSCSPI>;
}

//------

namespace psc::tft
{
    class LGFX : public lgfx::LGFX_Device
    {
        psc::io::SPI_sptr _bus_instance;

        lgfx::Panel_ST7735S _panel_instance;
        lgfx::Light_PWM _light_instance;

    public:
        LGFX(psc::io::SPI_sptr bus)
            : _bus_instance(bus)
        {
            {
                _panel_instance.setBus(_bus_instance.get());
            }

            {
                auto cfg = _panel_instance.config();

                cfg.pin_cs = TFT_CS;
                cfg.pin_rst = TFT_RST;
                cfg.pin_busy = TFT_BUSY;

                cfg.panel_width = 128;
                cfg.panel_height = 160;
                cfg.offset_x = 4;
                cfg.offset_y = 0;
                cfg.offset_rotation = 1;
                cfg.dummy_read_pixel = 8;
                cfg.dummy_read_bits = 1;
                cfg.readable = false;
                cfg.invert = false;
                cfg.rgb_order = true;
                cfg.dlen_16bit = false;
                cfg.bus_shared = true;

                _panel_instance.config(cfg);
            }

#if TFT_BL > 0
            {
                auto cfg = _light_instance.config();

                cfg.pin_bl = TFT_BL;
                cfg.invert = false;
                cfg.freq = 44100;
                cfg.pwm_channel = 7;

                _light_instance.config(cfg);
                _panel_instance.setLight(&_light_instance);
            }
#endif

            setPanel(&_panel_instance);
        }
    };

//------

    std::shared_ptr<LGFX> display;

    void setup(psc::io::SPI_sptr bus)
    {
        display = std::make_shared<LGFX>(bus);

        display->init();
#if TFT_BL > 0
        display->light()->setBrightness(128);
#endif
        display->setTextSize(0);
        display->setColorDepth(16);
        display->fillScreen(TFT_BLACK);
    }

    void __preDisplay(const bool clear)
    {
        display->startWrite(true);
        if (clear)
        {
            display->clearDisplay();
        }
        display->setTextDatum(BL_DATUM);
    }

    void __postDisplay()
    {
        display->endWrite();
    }

    void displayConfig(psc::config::Config &cfg)
    {
        __preDisplay(false);

        // More display-> calls in here

        __postDisplay(false);
    }
}

main


    psc::tft::setup(psc::io::SPI());
    psc::dac::setup();

DAC is being updated periodically in a timer task using psc::dac::writeDacOutput(...).

Then as soon as psc::tft::displayConfig(...) is called, the DAC no longer updates.

github-actions[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

LouDou commented 4 months ago

This issue should not be closed yet please.

lovyan03 commented 4 months ago

spi_bus_initialize はGFXが実行しているのでDACの方には不要ではないでしょうか

LouDou commented 4 months ago

@lovyan03 if I let this library call spi_bus_initialize then the DAC doesn't work. I need to have better control over initialising and using the bus myself for both the DAC and the display.

LouDou commented 3 months ago

I still haven't got to the bottom of what is going on here. There is a lot of complex code in lgfx's src/lgfx/v1/platforms/esp32/common.cpp interface, and it assumes in _spi_dev_handle that it owns the SPI bus and that only one device is ever on that bus. I therefore cannot let lgfx manage the bus and the transactions because it is unaware that other devices are on the bus.

This is a shame, I really like the lgfx library for graphics, I just wish it could play nicely with other devices.

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

LouDou commented 2 months ago

not stale

lovyan03 commented 2 months ago

検証しやすいように、ビルド可能なもっとシンプルなコードを提供してもらえませんか？

sukesh-ak commented 1 month ago

You can see the difference in code between Shared SPI bus and otherwise in ESP-IDF v5.x

    ESP_LOGI(TAG, "Initializing SPI BUS");
    spi_bus_config_t bus_cfg = {
        .mosi_io_num = SD_MOSI,
        .miso_io_num = SD_MISO,
        .sclk_io_num = SD_SCLK,
        .quadwp_io_num = -1,
        .quadhd_io_num = -1,
        .max_transfer_sz = 4092,
    };
    esp_err_t ret = spi_bus_initialize(SDSPI_HOST_ID, &bus_cfg, SDSPI_DEFAULT_DMA);
    if (ret != ESP_OK) {
        ESP_LOGE(TAG, "Failed to initialize bus.");
        return ESP_FAIL;
    }

Exclusive => https://github.com/sukesh-ak/ESP32-TUX/blob/master/main/helpers/helper_storage.hpp

Shared => https://github.com/sukesh-ak/ESP32-TUX/blob/master/main/helpers/helper_storage_shared.hpp

github-actions[bot] commented 3 weeks ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 2 weeks ago

This issue has been automatically closed because it has not had recent activity. Thank you for your contributions.

mhaberler commented 2 weeks ago

not stale.

lovyan03 / LovyanGFX