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

Fix display init sequence for Lilygo T-Display rp2040 and add missing 8 pins #8772

Merged
merged 13 commits into from
Jan 2, 2024

Conversation

kreier
Copy link

@kreier kreier commented Dec 30, 2023

The init sequence in board.c had one additional auto_brightness setting in the common_hal_busdisplay_busdisplay_construct() of the display_init() function that caused a one pixel noise line at the bottom of the display and a slow refresh rate from below 1 Hertz. Described in #8765

Removing this line solved the issue.

Furthermore the init sequence from the Adafruit Feather esp32s3 tft was copied to rotate the refresh of the display to "top to bottom" and have a final rotation=0 setting.

And the 8 missing pins in the pins.c were added.

kreier added 11 commits March 13, 2023 09:27
See pull request pidcodes/pidcodes.github.com#827

The original values from @erongd in https://github.com/erongd/circuitpython/blob/lilygo_t_display_rp2040/ports/raspberrypi/boards/lilygo_t_display_rp2040/mpconfigboard.mk point to a T-Display with esp32 (VID 0x303A is for espressif) but this board uses a raspberry pico (VID 0x2E8A) but it has not been granted for this product a PID. The pid.codes is a workaround
Update changes of the last month for new hardware models
The following 10 exposed pins were missing:
17
18
19
20
23
24
25 LED
(26 - is VOLTAGE_MONITOR)
27
28
29
The auto brightness setting caused a slow refresh rate below 1 Hz and a pixel noise error of one pixel height below the display. Removing this line fixed this issue.

Further the init sequence from the Adafruit Feather esp32s3 tft was copied over with _MADCTL 0x36, 1, 0x68 and adjusted column and row start. The rotation is now 0 so the display is refreshed from top to bottom.
jepler
jepler previously approved these changes Dec 30, 2023
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

looks good, no HW for testing.

90, // rotation
40, // column start
53, // row start
0, // rotation
Copy link
Member

Choose a reason for hiding this comment

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

Configurations which have a "0 degree" rotation of the displayio display are almost always preferable.

@@ -116,13 +116,13 @@ static void display_init(void) {
&pin_GPIO4, // backlight pin
NO_BRIGHTNESS_COMMAND,
1.0f, // brightness (ignored)
false, // auto_brightness
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised the display worked at all with the argument positions shifted like this!

The initial setting was RGB and is used for the T-Display ESP32 as well.
@kreier
Copy link
Author

kreier commented Dec 30, 2023

Sorry, not dismissed. But I found one error that was not obvious with B/W text but with the use of color. The D3 bit of MADCTL changes the color information. It was initially zero and therefore RGB but with the change to 0x68 we set it to one and therefore BGR. The change to 0x60 switches the order back to RGB and is consistent with the use in T-Display ESP32. So the change from the original 0xC0 only changes the orientation and refresh direction to allow for rotation=0

@kreier
Copy link
Author

kreier commented Jan 1, 2024

Sorry, my mistake. The correct value for MADCTL is 0x68. I checked three values in different builds and the impact on the screen:

  • 0x08 is the old value used prior to this pull request. colors are fine, refresh from left to right
  • 0x60 flips the D3 bit of MADCTL. Colors created by adafruit_display_text are not affected, RGB values submitted with the color attribute in Label render the RGB color as intended and before. But colors used as color_palette[0] in displayio.Palette(1) and applied to displayio.TileGrid as pixel_shader argument no longer represent RGB.
  • 0x68 has the correct colors and draws from top to bottom

Now I have to find a way to retract my last commit.

It looks like general consensus to have D3 set to 1 across different boards with this display and the ST7789:

board MADCTL _MADCTL rotation column start row start width height
T-Display rp2040 0x08 0b00001000 90 53 40 240 135
T-Display ESP32 16M 0x08 0b00001000 270 53 40 240 135
Feather ESP32-S2 with TFT 0x68 0b01101000 0 40 53 240 135
Feather ESP32-S2 reverse TFT 0x68 0b01101000 0 40 53 240 135
TTGO T8 ESP32-S2 ST7789 0x08 0b00001000 90 52 40 240 135
Lilygo T-Embed ESP32-S3 0xC8 0b11001000 90 35 0 320 170
MEMENTO 0xA0 0b10100000 0 80 0 240 240
CLUE NRF52840 Express 0xA0 0b10100000 0 80 0 240 240
Lilygo T-Deck 0x08 0b00001000 270 0 0 320 240
M5StickC PLUS (ST7735R) 0x08 0b00001000 1 40 52 135 240

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Thank you for the extended testing.

@jepler jepler merged commit a21a71c into adafruit:main Jan 2, 2024
14 checks passed
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.

2 participants