Skip to content

fix(m5stack_tab5): support ST7121 LCD variant#804

Open
RiviaRammer wants to merge 5 commits into
espressif:masterfrom
RiviaRammer:fix/m5stack-tab5-st7121
Open

fix(m5stack_tab5): support ST7121 LCD variant#804
RiviaRammer wants to merge 5 commits into
espressif:masterfrom
RiviaRammer:fix/m5stack-tab5-st7121

Conversation

@RiviaRammer

@RiviaRammer RiviaRammer commented Jul 10, 2026

Copy link
Copy Markdown

问题背景

M5Stack Tab5 硬件存在不同 LCD 方案。当前 BSP 在 I2C 地址 0x55 探测到 ST712x 触摸控制器后,会统一按 ST7123 LCD 初始化。

但实际 Tab5 UserDemo 中会读取 ST712x 寄存器 0x0000 的固件版本来区分 LCD 方案:

  • FW version = 1: ST7121
  • FW version = 3: ST7123

因此,使用 ST7121 屏幕方案的 Tab5 在当前 BSP 下无法正确初始化显示。

修改内容

  • 新增 esp_lcd_st7121 LCD panel component
  • m5stack_tab5 BSP 中读取 ST712x FW version,用于区分 ST7121 / ST7123
  • FW version = 1 时使用 ST7121 LCD 初始化流程
  • FW version = 3 时保持 ST7123 LCD 初始化流程
  • 保留原有 GT911 / ILI9881C 路径
  • 更新 Tab5 BSP 的依赖声明和 README 能力表

测试

已在一台 ST7121 版本的 M5Stack Tab5 上验证:

  • BSP 检测结果为 ST7121 / FW 1
  • 使用独立 bring-up demo 测试 RGB 三原色循环显示正常
  • 彩条显示正常
  • git diff --check 无格式错误

未实机回归测试 ST7123 / ILI9881C 版本。


Note

Medium Risk
Changes runtime board detection and MIPI/LCD init for all Tab5 ST712x units; ST7123 and ILI9881C paths are intended to be unchanged but were not fully regression-tested on hardware per the PR description.

Overview
Adds ST7121 as a third M5Stack Tab5 display variant so boards that share the ST712x touch I2C address no longer always initialize as ST7123.

Board detection in bsp_display.c now uses a typed enum and, when the ST712x touch is present, reads touch firmware version from register 0x0000: FW 1 → ST7121, FW 3 → ST7123 (unchanged ILI9881C + GT911 path). Unknown FW versions fail with ESP_ERR_NOT_SUPPORTED instead of assuming ST7123.

Display bring-up selects esp_lcd_new_panel_st7121 with ST7121-specific MIPI DPI timing, default init commands in the new driver, and a fixed 965 Mbps DSI lane rate for ST7121. Touch for ST7121 reuses the existing ST7123 touch driver.

A new local component components/lcd/esp_lcd_st7121 (MIPI DSI panel driver) is wired into Tab5 via idf_component.yml / m5stack_tab5.json; root and Tab5 README capability tables list st7121.

Reviewed by Cursor Bugbot for commit af2e35d. Bugbot is set up for automated code reviews on this repo. Configure here.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread bsp/m5stack_tab5/src/bsp_display.c

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 453faf1. Configure here.

Comment thread bsp/m5stack_tab5/src/bsp_display.c Outdated
Comment thread bsp/m5stack_tab5/src/bsp_display.c
@espzav

espzav commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

HI @RiviaRammer thank you for this PR. Great job. I have one question:

  • I compared esp_lcd_st7123.c and esp_lcd_st7121.c and there is not too much difference, if I understand, there are only another init commands, right? Can we use the st7123 driver for both? I haven't got HW with st7121, I cannot test it.

@RiviaRammer

Copy link
Copy Markdown
Author

HI @RiviaRammer thank you for this PR. Great job. I have one question:

  • I compared esp_lcd_st7123.c and esp_lcd_st7121.c and there is not too much difference, if I understand, there are only another init commands, right? Can we use the st7123 driver for both? I haven't got HW with st7121, I cannot test it.

Thank you for reviewing.

The two panel drivers share a very similar MIPI-DPI wrapper, but they do not have the same default initialization sequence. In particular, the ST7121 sequence uses different vendor commands (for example, the 0x60 command starts with 0x71, 0x21 rather than 0x71, 0x23), and the ST7121 timing also differs.

I kept a separate ST7121 driver because M5Stack's Tab5 UserDemo follows the same approach: it detects touch FW version 1, creates esp_lcd_st7121, and uses the ST7121-specific default init sequence. I verified this path on an FW-version-1 Tab5 unit with a stable RGB/color-bar test.

Using the current ST7123 driver unchanged for both would select the ST7123 default initialization sequence for ST7121. It could be refactored into a shared ST712x driver in a separate change, but that would be a broader component API change.

@espzav

espzav commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

but they do not have the same default initialization sequence

If the difference is only in init sequence, it can be same like this:

const st7123_vendor_config_t vendor_config_st7123 = {
        .init_cmds      = disp_init_data_st7123,
        .init_cmds_size = sizeof(disp_init_data_st7123) / sizeof(disp_init_data_st7123[0]),

I kept a separate ST7121 driver because M5Stack's Tab5 UserDemo follows the same approach: it detects touch FW version 1, creates esp_lcd_st7121, and uses the ST7121-specific default init sequence.

The detection can be still same. Detection is only reading register from I2C.

As I said before, I haven't got HW and it is not available now. But I would test new init commands with esp_lcd_st7123 - if it will work, we don't need new driver.

@imliubo

imliubo commented Jul 17, 2026

Copy link
Copy Markdown

I'd like to help move this PR forward toward merging. I'll arrange for one of our engineers to test it and share the test results here.

Thank you, @RiviaRammer, for your support, and @espzav for the review.

@Forairaaaaa

Copy link
Copy Markdown

All three display models tested ok.

e10745c0-c50f-45ef-82ae-794a0c96968e d83e5324-b4c6-44ee-adef-a0a84ef4f55f 0d94bce1-882e-4c2c-8d04-4f26c4ed1a35

@espzav

espzav commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

@Forairaaaaa Thank you for tests. Please, could you give me more info, how did you test it?
I would like to see, that display ST7121 will work with driver ST7123 (only with changed vendor config data - init commands). I think, it should work.

Thank you!

@Forairaaaaa

Copy link
Copy Markdown

@Forairaaaaa Thank you for tests. Please, could you give me more info, how did you test it? I would like to see, that display ST7121 will work with driver ST7123 (only with changed vendor config data - init commands). I think, it should work.

Thank you!

Thank you for the clarification.

For my previous test, I built the examples/display example from the current PR head and flashed the same binary to all three Tab5 display variants. I visually checked that each device was detected correctly and that the complete LVGL startup animation was displayed without artifacts or crashes.

However, that test used the implementation in this PR as-is. On the ST7121 unit it called esp_lcd_new_panel_st7121(), so it did not verify whether ST7121 can be driven by esp_lcd_st7123.

I now understand the test you are requesting: keep the existing runtime detection, use esp_lcd_new_panel_st7123() for both ST7121 and ST7123, and provide the ST7121-specific init commands and DPI configuration through st7123_vendor_config_t.

I have not tested that configuration yet. I will prepare a build using this approach, test it on the ST7121 hardware, and report the result here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants