ST7789 Static Noise displaying to screen during initialization

tvanfossen commented 3 years ago

ST7789 initialization seems out of date.

In LVGL @ master, LVGL v7 and LVGL v8 the same issue is present, where static noise is displayed to the screen during initialization in st7789.c

I've tried a few different changes to the command list with no luck.

The driver commands seem out of date with documentation found here. https://www.rhydolabz.com/documents/33/ST7789.pdf

There are a slew of commands being past to the init that do not exist in the documentation as far as I can tell, and ones that have no associated define in st7789.h. With these undefined commands commented out, the problem of static noise on the screen remains.

Any help would be much appreciated! The static noise kinda kills this driver for the project I am working on, as it shows on every bootup, and in this case the display is on/off often

C47D commented 3 years ago

I think I have that display at hand, let me confirm that and I will let you know.

On the meantime, can you upload the sdkconfig.h file available in build/config? You can attach it to your comment. Also, how are you wiring the display with the MCU and what MCU are you using?

tore-espressif commented 3 years ago

@tvanfossen can you control LCD's backlight from the MCU?

The easiest way is to enable the backlight only after you have transferred color data to the display, so there is always valid data in the Graphics RAM

tvanfossen commented 3 years ago

Co-worker was able to get the issue resolved inside of st7789.c - will either let him update or get the fix from him

MCU does not have backlight control, that was the first idea for a quick fix

On a custom board with an esp32, I dont have the wiring diagram at hand

sdkconfig.zip

C47D commented 3 years ago

Hi @tvanfossen , I've found my display and ESP32 kit, and I'm able to flash the demo, I'm not able to see the static noise (I'm not using the custom display buffer size tho), are you using a buyed display?

My pc is pretty slow, it takes about 15mins to build a project, so I will be patient to try find the issue 😅

C47D commented 3 years ago

So I've used the custom display buffer size and I'm not seeing any issues, here's my sdkconfig file in case you want to take a look at it. And a bit from the monitor output:

I (406) cpu_start: Starting scheduler on PRO CPU.
I (0) cpu_start: Starting scheduler on APP CPU.
I (30) lvgl_helpers: Display hor size: 240, ver size: 240
I (40) lvgl_helpers: Display buffer size: 4096
I (40) lvgl_helpers: Initializing SPI master for display
I (50) lvgl_helpers: Configuring SPI host VSPI_HOST (2)
I (50) lvgl_helpers: MISO pin: -1, MOSI pin: 23, SCLK pin: 18, IO2/WP pin: -1, IO3/HD pin: -1
I (60) lvgl_helpers: Max transfer size: 8192 (bytes)
I (70) lvgl_helpers: Initializing SPI bus...
I (70) disp_spi: Adding SPI device
I (70) disp_spi: Clock speed: 20000000Hz, mode: 2, CS pin: -1
ST7789 initialization.
I (480) st7789: Display orientation: PORTRAIT
I (480) st7789: 0x36 command value: 0xC0

sdkconfig.txt

jparker324 commented 3 years ago

@C47D I work with @tvanfossen. In order to clear the display memory, I've had to replace Lines 78-80 of the st7789.c file :

{ST7789_CASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_RASET, {0x00, 0x00, 0x01, 0x3f}, 4},
{ST7789_RAMWR, {0}, 0},

with a longer sequence of memory writes

{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x00, 0x00, 0x10}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x11, 0x00, 0x21}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x22, 0x00, 0x32}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x33, 0x00, 0x43}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x44, 0x00, 0x54}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x55, 0x00, 0x65}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x66, 0x00, 0x76}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x77, 0x00, 0x87}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x88, 0x00, 0x98}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x99, 0x00, 0xA9}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0xAA, 0x00, 0xBA}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0xBB, 0x00, 0xCB}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0xCC, 0x00, 0xDC}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0xDD, 0x00, 0xED}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0xEE, 0x00, 0xFE}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0xFF, 0x01, 0x0F}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x01, 0x10, 0x01, 0x20}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x01, 0x21, 0x01, 0x31}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x01, 0x32, 0x01, 0x3f}, 4},
{ST7789_RAMWR, {0}, 0},

and then add some handling for the ST7789_RAMWR command, such that the data payload is taken from a 4080 bytes buffer filled with the desired default color.

For this, I replaced the code around Line 109-118 with the following.

#define DISP_BUF_SIZE 4080

lv_color_t color_map[DISP_BUF_SIZE];
for(uint16_t i=0; i < DISP_BUF_SIZE; i++)
{
    color_map[i].full = 0xA310;  //grey color
    // color_map[i].full = 0x0000;  //black (memset would be faster)
}

//Send all the commands
uint16_t cmd = 0;
while (st7789_init_cmds[cmd].databytes!=0xFF) {
    if (st7789_init_cmds[cmd].cmd != ST7789_RAMWR)
    {
        st7789_send_cmd(st7789_init_cmds[cmd].cmd);
        st7789_send_data(st7789_init_cmds[cmd].data, st7789_init_cmds[cmd].databytes&0x1F);
        if (st7789_init_cmds[cmd].databytes & 0x80) {
                vTaskDelay(100 / portTICK_RATE_MS);
        }
    }
    else
    {
        st7789_send_cmd(st7789_init_cmds[cmd].cmd);
        st7789_send_data((void *)&color_map, DISP_BUF_SIZE * 2);
    }
    cmd++;
}

And not to detract from the core issue, but can you explain the purpose of the commands in st7789_init_cmds that aren't enumerated? See Line 54 for example. The commands aren't defined in any ST7789 driver documentation I can find and removing them seems to have no effect.

jparker324 commented 3 years ago

@C47D Alternatively, I can also initialize the display successfully by sending the full address range once (as the driver currently does), but then use repeated ST7789_RAMWRC commands with the previously mentioned 4080 byte color data payload

{ST7789_RASET, {0x00, 0x00, 0x00, 0xEF}, 4},
{ST7789_CASET, {0x00, 0x00, 0x01, 0x3F}, 4},
{ST7789_RAMWR, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},
{ST7789_RAMWRC, {0}, 0},

C47D commented 3 years ago

Hi @jparker324 , thanks for reaching out, I understand your issue now, you mean the "noise" when the display is powered up, with the code you sent you're "clearing" the display before initializing it, so once it starts up it dosn't show the noise, isn't?

Which of your approaches feels more correct to you? I think we could initialize the color_map array to 0 with this:

#define DISP_BUF_SIZE 4080

lv_color_t color_map[DISP_BUF_SIZE] = {0};

And not to detract from the core issue, but can you explain the purpose of the commands in st7789_init_cmds that aren't enumerated? See Line 54 for example. The commands aren't defined in any ST7789 driver documentation I can find and removing them seems to have no effect.

To be honest I don't remember where those values came from, we can remove them if they aren't necessary.

jparker324 commented 3 years ago

@C47D Yes, that understanding is correct. I think initializing the color to black is fine. We were using a grey here to avoid painting the whole screen one additional time, but black would be better for a shared driver init color in any case. I've made the following changes and my current test code is below.

Removed unknown commands from st7789_init_cmds
Corrected color_map initialization per your recommendation
Removed delay not required for ST7789_DISPON
Reduced delay for ST7789_SLPOUT from 100ms to 5ms per specification (the actual observed minimum possible delay seems to be 16ms probably due to some timer or call rate?)
Adjusted DISP_BUF_SIZE multiplier when used in st7789_send_data() to reflect the size of the color variable (for 8- 16- 32- bit colors) (untested)
Comments clean-up: removed enumerated command numbers and added addresses for ST7789_RAMWR and ST7789_RAMWRC commands

    lcd_init_cmd_t st7789_init_cmds[] = {
        {ST7789_PWCTRL2,    {0x85, 0x01, 0x79}, 3},
        {ST7789_LCMCTRL,    {0x26}, 1},
        {ST7789_IDSET,      {0x11}, 1},
        {ST7789_VCMOFSET,   {0x35, 0x3E}, 2},
        {ST7789_CABCCTRL,   {0xBE}, 1},
        {ST7789_MADCTL,     {0x00}, 1}, // Set to 0x28 if your display is flipped
        {ST7789_COLMOD,     {0x55}, 1},
#if ST7789_INVERT_COLORS == 1
        {ST7789_INVON,      {0x00}, 0}, // set inverted mode
#else
        {ST7789_INVOFF,     {0x00}, 0}, // set non-inverted mode
#endif
        {ST7789_RGBCTRL,    {0x00, 0x1B}, 2},
        {ST7789_GAMSET,     {0x01}, 1},
        {ST7789_PVGAMCTRL,  {0xD0, 0x00, 0x02, 0x07, 0x0A, 0x28, 0x32, 0x44, 0x42, 0x06, 0x0E, 0x12, 0x14, 0x17}, 14},
        {ST7789_NVGAMCTRL,  {0xD0, 0x00, 0x02, 0x07, 0x0A, 0x28, 0x31, 0x54, 0x47, 0x0E, 0x1C, 0x17, 0x1B, 0x1E}, 14},
        {ST7789_RASET,      {0x00, 0x00, 0x00, 0xEF}, 4},
        {ST7789_CASET,      {0x00, 0x00, 0x01, 0x3F}, 4},
        {ST7789_RAMWR,      {0x00}, 0}, // 0x0000-0x0010
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0011-0x0021
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0022-0x0032
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0033-0x0043
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0044-0x0054
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0055-0x0065
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0066-0x0076
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0077-0x0087
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0088-0x0098
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0099-0x00A9
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x00AA-0x00AB
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x00BB-0x00BC
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x00CC-0x00CD
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x00DD-0x00DE
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x00EE-0x00EF
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x00FF-0x010F
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0110-0x0120
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0121-0x0131
        {ST7789_RAMWRC,     {0x00}, 0}, // 0x0132-0x013F  partial write
        {ST7789_GCTRL,      {0x07}, 1},
        {ST7789_SLPOUT,     {0x00}, 0x80}, //requires 5ms delay after send
        {ST7789_DISPON,     {0x00}, 0},
        {ST7789_NOP,        {0x00}, 0xFF} // not sent, end-of-list
    };

and

    printf("ST7789 initialization.\n");

    #define DISP_BUF_SIZE 4080

    lv_color_t color_map[DISP_BUF_SIZE] = {0};

    //Send all the commands
    uint16_t cmd = 0;
    while (st7789_init_cmds[cmd].databytes!=0xFF) {
        if ((st7789_init_cmds[cmd].cmd != ST7789_RAMWR) && (st7789_init_cmds[cmd].cmd != ST7789_RAMWRC))
        {
            st7789_send_cmd(st7789_init_cmds[cmd].cmd);
            st7789_send_data(st7789_init_cmds[cmd].data, st7789_init_cmds[cmd].databytes&0x1F);
            if (st7789_init_cmds[cmd].databytes & 0x80) {
                vTaskDelay(5 / portTICK_RATE_MS);
            }
        }
        else
        {
            st7789_send_cmd(st7789_init_cmds[cmd].cmd);
            st7789_send_data((void *)&color_map, DISP_BUF_SIZE * sizeof(lv_color_t));
        }
        cmd++;
    }

    st7789_set_orientation(CONFIG_LV_DISPLAY_ORIENTATION);

C47D commented 3 years ago

It does look good to me, are you planning to submit your changes into a PR? I would only rename the DISP_BUF_SIZE symbol, afaik it's not related to the size allocated for the LVGL work buffer, isn't? I'm also working on #92 , which is using the same display driver, if you need me to test your changes please let me know.

lvgl / lvgl_esp32_drivers

ST7789 Static Noise displaying to screen during initialization #104