Skip to content

fix(m5stack_tab5): support ST7121 LCD variant - #804

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

fix(m5stack_tab5): support ST7121 LCD variant#804
RiviaRammer wants to merge 8 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 的固件版本区分屏幕:

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

因此,ST7121 屏幕版本需要使用不同的初始化命令和 DPI 时序。

修改内容

  • 读取 ST712x FW version,区分 ST7121 和 ST7123
  • 保留原有 GT911 / ILI9881C 路径
  • ST7121 和 ST7123 均使用现有 esp_lcd_new_panel_st7123() driver
  • ST7121 使用其专用初始化命令、965 Mbps DSI lane rate 和 DPI timing
  • 不新增独立的 esp_lcd_st7121 component

测试

  • 已使用 ESP-IDF v5.4.3 重新构建 Tab5 bring-up demo
  • ELF 仅包含 esp_lcd_new_panel_st7123,不包含 esp_lcd_new_panel_st7121
  • 官方工程师已使用此实现方式在实体 ST7121 和 ST7123 Tab5 上测试,显示动画和触摸正常,未见花屏、闪烁或重启
  • git diff --check 通过

Note

Medium Risk
Changes auto-detection and MIPI/DPI init for all Tab5 ST712x boards; mis-read FW versions could block display init, though ST7123/ST7121 paths were validated on hardware.

Overview
Adds ST7121 as a third Tab5 hardware revision alongside ILI9881C/GT911 and ST7123, instead of treating every ST712x touch probe as ST7123.

Board detection now reads the ST712x touch firmware byte at register 0x0000 after I2C probe: FW 1 selects ST7121, FW 3 keeps ST7123; unknown values fail detection instead of assuming one panel. Magic 1/2 integers are replaced with a bsp_tab5_board_version_t enum, and display/touch setup bail out with ESP_ERR_NOT_SUPPORTED when detection fails.

ST7121 display path adds disp_init_data_st7121.h, wires it through the existing esp_lcd_new_panel_st7123() driver with ST7121-specific init commands and DPI porch/timing, and raises MIPI DSI lane bitrate to 965 Mbps for that variant in the LVGL display start path. Touch for ST7121 shares the ST7123 I2C touch driver.

Component version is bumped to 1.3.0.

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

@CLAassistant

CLAassistant commented Jul 10, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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 Outdated
@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.

@espzav

espzav commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@Forairaaaaa Hi, please do you have any update here?

@Forairaaaaa

Copy link
Copy Markdown

@Forairaaaaa Hi, please do you have any update here?

Hi, I tested this with a local change on top of af2e35d.

Both ST7121 and ST7123 use esp_lcd_new_panel_st7123(). For ST7121, I passed its original initialization commands through st7123_vendor_config_t, while keeping the ST7121-specific DSI lane rate and DPI timing. I also removed the esp_lcd_st7121 dependency and verified that the final ELF contains esp_lcd_new_panel_st7123, but not esp_lcd_new_panel_st7121.

I built examples/display with ESP-IDF v5.4.4 and flashed it to physical Tab5 boards with ST7121 and ST7123 panels. On both boards, the display initialized correctly, the animation rendered normally, and touch input worked. I did not observe any artifacts, flickering, or unexpected resets.

So yes, ST7121 works with the ST7123 driver when using the ST7121-specific vendor initialization commands.

@espzav

espzav commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

So yes, ST7121 works with the ST7123 driver when using the ST7121-specific vendor initialization commands.

@Forairaaaaa Thank you very much for help!

@RiviaRammer Please, could you rework this PR by the latest information about driver?

@RiviaRammer

Copy link
Copy Markdown
Author

Thank you for the additional hardware validation. I have reworked this PR accordingly.

The separate esp_lcd_st7121 component and its dependency have been removed. The ST7121-specific initialization commands are now kept in the Tab5 BSP, while both ST7121 and ST7123 use esp_lcd_new_panel_st7123() with their respective st7123_vendor_config_t settings.

The existing runtime detection is retained. ST7121 also keeps its specific 965 Mbps DSI lane rate and DPI timing. I rebuilt the Tab5 bring-up demo successfully, and its ELF contains esp_lcd_new_panel_st7123 only.

@espzav espzav left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@RiviaRammer Thank you for changes. It looks good, I left some minor comments. After your changes, I will check it on HW yet.

Comment thread bsp/m5stack_tab5/idf_component.yml Outdated
Comment thread bsp/m5stack_tab5/src/bsp_display.c Outdated
RiviaRammer and others added 2 commits August 29, 2026 00:39
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