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

The problem of esp_lcd handling color data (IDFGH-8881) #10302

Closed
3 tasks done
lbuque opened this issue Dec 3, 2022 · 5 comments
Closed
3 tasks done

The problem of esp_lcd handling color data (IDFGH-8881) #10302

lbuque opened this issue Dec 3, 2022 · 5 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@lbuque
Copy link

lbuque commented Dec 3, 2022

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

I'm compiling micropython's binding library for esp_lcd.

I consider that esp32 may not have psram, so moving data to esp_lcd line by line has saved memory overhead.

The result I get is that the lcd doesn't display my image properly.

Below is my code to initialize the lcd panel:

⚠️ When I use a method like full brush to transfer data, it is displayed normally!

How should I use esp_lcd_panel_io_tx_color correctly?

Answers from anyone are welcome!

void hal_lcd_spi_panel_construct(lcd_spi_panel_obj_t *self) {
    machine_hw_spi_obj_t *spi_obj = ((machine_hw_spi_obj_t *)self->spi_obj);
    if (spi_obj->state == MACHINE_HW_SPI_STATE_INIT) {
        spi_obj->state = MACHINE_HW_SPI_STATE_DEINIT;
        machine_hw_spi_deinit_internal(spi_obj);
    }
    spi_bus_config_t buscfg = {
        .sclk_io_num = spi_obj->sck,
        .mosi_io_num = spi_obj->mosi,
        .miso_io_num = -1,
        .quadwp_io_num = -1,
        .quadhd_io_num = -1,
// #if CONFIG_ESP32_SPIRAM_SUPPORT
        .max_transfer_sz = self->width * self->height * 2 + 8,
// #else
//         .max_transfer_sz = self->width * 2 + 8,
// #endif
    };
    esp_err_t ret = spi_bus_initialize(spi_obj->host, &buscfg, SPI_DMA_CH_AUTO);
    if (ret != 0) {
        mp_raise_msg_varg(&mp_type_OSError, "%d(spi_bus_initialize)", ret);
    }
    spi_obj->state = MACHINE_HW_SPI_STATE_INIT;

    esp_lcd_panel_io_spi_config_t io_config = {
        .dc_gpio_num = mp_hal_get_pin_obj(self->dc),
        .cs_gpio_num = mp_hal_get_pin_obj(self->cs),
        .pclk_hz = self->pclk,
        .lcd_cmd_bits = self->cmd_bits,
        .lcd_param_bits = self->param_bits,
        .spi_mode = 0,
        .trans_queue_depth = 10,
    };

    // Attach the LCD to the SPI bus
    ret = esp_lcd_new_panel_io_spi(
        (esp_lcd_spi_bus_handle_t)spi_obj->host,
        &io_config,
        &self->io_handle
    );
    if (ret != 0) {
        mp_raise_msg_varg(&mp_type_OSError, "%d(esp_lcd_new_panel_io_spi)", ret);
    }
}

Here's how to move color data:

inline void hal_lcd_spi_panel_tx_color(
    lcd_spi_panel_obj_t *self,
    int lcd_cmd,
    const void *color,
    size_t color_size
) {
    DEBUG_printf("tx_color cmd: %x, color_size: %u\n", lcd_cmd, color_size);

    size_t i = 1;
    esp_lcd_panel_io_tx_color(self->io_handle, lcd_cmd, NULL, 0);
    for (i = 0; i < 135; i++) {
        esp_lcd_panel_io_tx_color(
            self->io_handle,
            -1,
            (const void *)&((const uint8_t *)color)[i * 480],
            480
        );
    }
    esp_lcd_panel_io_tx_color(
        self->io_handle,
        -1,
        (const void *)&((const uint8_t *)color)[i * 480],
        color_size - (i * 480)
    );

}
@espressif-bot espressif-bot added the Status: Opened Issue is new label Dec 3, 2022
@github-actions github-actions bot changed the title The problem of esp_lcd handling color data The problem of esp_lcd handling color data (IDFGH-8881) Dec 3, 2022
@atanisoft
Copy link

Is there a reason not to use esp_lcd_panel_draw_bitmap sending pixel data line by line? Using this API sends the column/line offsets to the LCD prior to sending the block of pixel data.

By directly using esp_lcd_panel_io_tx_color it is bypassing the column/line offsets being sent to the LCD which could be why it doesn't display correctly.

Your first usage of esp_lcd_panel_io_tx_color should likely be esp_lcd_panel_io_tx_param if it is a command being sent that requires the usage of the CD pin.

@lbuque
Copy link
Author

lbuque commented Dec 4, 2022

@atanisoft
I have used esp_lcd_panel_io_tx_param to set column/line offsets before using esp_lcd_panel_io_tx_color.

esp_lcd_panel_draw_bitmap is just a code block of esp_lcd_panel_io_tx_color and esp_lcd_panel_io_tx_param according to different driver ic.

Before moving each line of color data, I set the column/line offsets on the LCD, and it is displayed normally.

for (size_t i = 0; i < (y_end - y_start); i++) {
    esp_lcd_panel_io_tx_param(
        self->io_handle,
        0x2A, 
        (uint8_t[]) {
            ((x_start >> 8) & 0xFF),
            (x_start & 0xFF),
            (((x_end - 1) >> 8) & 0xFF),
            ((x_end - 1) & 0xFF),
        },
        4
    );

    esp_lcd_panel_io_tx_param(
        self->io_handle,
        0x2B,
        (uint8_t[]) {
            (((y_start + i) >> 8) & 0xFF),
            ((y_start + i) & 0xFF),
            (((y_start + i + 1) >> 8) & 0xFF),
            ((y_start + i + 1) & 0xFF),
         }, 
        4
    );

    size_t len = (x_end - x_start) * self->bits_per_pixel / 8;
    esp_lcd_panel_io_tx_color((lcd_spi_panel_obj_t *)self->bus_obj, 0x2C, &bufinfo.buf[i * len], len);
}

When I change the code to the following way, it doesn't work:

esp_lcd_panel_io_tx_param(
    (lcd_spi_panel_obj_t *)self->bus_obj,
    0x2A, 
    (uint8_t[]) {
        ((x_start >> 8) & 0xFF),
        (x_start & 0xFF),
        (((x_end - 1) >> 8) & 0xFF),
        ((x_end - 1) & 0xFF),
    }, 
    4
);

esp_lcd_panel_io_tx_param(
    (lcd_spi_panel_obj_t *)self->bus_obj,
    0x2B, 
    (uint8_t[]) {
        (((y_start) >> 8) & 0xFF),
        ((y_start ) & 0xFF),
        (((x_end  -1) >> 8) & 0xFF),
        ((x_end  -1) & 0xFF),
    },
    4
   );

esp_lcd_panel_io_tx_color(self->io_handle, 0x2C, NULL, 0);
for (i = 0; i < (y_end - y_start ); i++) {
    esp_lcd_panel_io_tx_color(
        self->io_handle,
        -1,
        (const void *)&((const uint8_t *)color)[i * (x_end - x_start ) * 2],
        (x_end - x_start ) * 2
    );
}
esp_lcd_panel_io_tx_color(
    self->io_handle,
    -1,
    (const void *)&((const uint8_t *)color)[i * (x_end - x_start ) * 2],
    color_size - (i * (x_end - x_start ) * 2)
);

I looked at the code implementation of esp_lcd_panel_io_tx_color, it seems that even if the value of lcd_cmd is -1, it will send -1 to the lcd. The document describes that when lcd_cmd is not needed, set lcd_cmd to -1. I guess it may be that the color data cannot be correctly transported and matched with the LCD.

image

@suda-morris
Copy link
Collaborator

@lbuque if you want to send a single command, suggest using esp_lcd_panel_io_tx_param.

I tested the following code, it works on my ST7789 panel (master branch, but I think it should have the same behavior on 5.0)

    // define an area of frame memory where MCU can access
    esp_lcd_panel_io_tx_param(io, LCD_CMD_CASET, (uint8_t[]) {
        (x_start >> 8) & 0xFF,
        x_start & 0xFF,
        ((x_end - 1) >> 8) & 0xFF,
        (x_end - 1) & 0xFF,
    }, 4);
    esp_lcd_panel_io_tx_param(io, LCD_CMD_RASET, (uint8_t[]) {
        (y_start >> 8) & 0xFF,
        y_start & 0xFF,
        ((y_end - 1) >> 8) & 0xFF,
        (y_end - 1) & 0xFF,
    }, 4);
    // transfer frame buffer
    esp_lcd_panel_io_tx_param(io, LCD_CMD_RAMWR, NULL, 0);
    size_t len = (x_end - x_start) * st7789->fb_bits_per_pixel / 8;
    uint8_t *data = (uint8_t *) color_data;
    for (int i = 0; i < y_end - y_start; i++) {
        esp_lcd_panel_io_tx_color(io, -1, &data[i * len], len);
    }

the code implementation of esp_lcd_panel_io_tx_color, it seems that even if the value of lcd_cmd is -1, it will send -1 to the LCD.

Yes, seems like some commits in #9881 need to be backported to the 4.4 branch to allow lcd_cmd set to -1

@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed and removed Status: Opened Issue is new Status: In Progress Work is in progress labels Dec 5, 2022
@lbuque
Copy link
Author

lbuque commented Dec 6, 2022

@suda-morris Thank you very much for your answer!

At present, micropython is not compatible with idf 5.0, I can't verify it, but it must be correct.

So now I just have to wait for it to be backported to the 4.4 branch.

Hope you can let me know after the backport!

I think this issue can be closed now!

@espressif-bot espressif-bot added Resolution: Done Issue is done internally Status: Done Issue is done internally and removed Status: Reviewing Issue is being reviewed labels Dec 6, 2022
espressif-bot pushed a commit that referenced this issue Dec 9, 2022
@Alvin1Zhang
Copy link
Collaborator

Thanks for sharing the updates, the backport on release/4.4 is available at ca6a427, feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

5 participants