[Bug] Some TFT configurations (and possibly other GPIO action) has problems with large GPIO #'s

[robertlipe]

This doesn't actually affect me. It's my civic duty to report it and help analyze it because the next person to be affected by it may not be as mellow about it. I'm just killing time waiting for stupid python in stupid platformio to burn two minutes to analyze my stupid dependencies the 11 billionth's time before it performs a two second stupid action. (Hmm. mybe I should work on that mellow... On with the show!)

Describe the bug
The LilyGo configuration, like many that are possible on the ESP32S3 which has something like 45 GPIO registers has register numbers above 31 and thus, incapable of fitting into a processor word.

To Reproduce
Steps to reproduce the behavior:
~/.platformio/penv/bin/pio run --upload-port $PORT --target upload -e lilygo-t-display-s3

Hold your eyes open and don't blink when the yellow text scrolls by.

Expected behavior
Glorious, warning-free compilation.

Screenshots
If applicable, add screenshots to help explain your problem.

Please complete the following information:
~/.platformio/penv/bin/pio --version
PlatformIO Core, version 6.1.9

Additional context

Catch one warning in isolation and it's "obvious" what's going on.

.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c: In member function 'uint8_t TFT_eSPI::readByte()':
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:115:16: note: in expansion of macro 'TFT_D0'
b = (((reg>>TFT_D0)&1) << 0);

Shifting anything 39 bits in a 32-bit processor isn't going to fly. This is a really common problem in embedded-land that we're starting to see even on the big-boys. The JH-7110 has some internally remapped GPIO numbers that are larger than 64-bits (!) so even that code needs reworked when ported to these newfangled SoCs with their crazy-axe register configuration blocks.

I think that's technically undefined behaviour; some systems will shift all the bits through and give a zero. Some will do a rotate (exactly what this case DOESN'T need) and some systems will toss an execution exception and stall. If these shifts are used in address calculations (common) it can result in memory in the wrong place being scribbled on. That's clearly bad.

I think this is an upstream library. If it's Bodmer TFT, there's surely an update that addresses this because it seems unlikely we're the first to run into this. That style of code (The PIO abstraction over the Arduino abstraction over the IDF abstraction for a #define for a number from 1-256) for something that compiles into one opcode can be depressingly hard to pencil-whip and that SPI - style of code where cmd might be an extra bit or an extra store completely can just be a mess to work through.

If you're lucky, someone else already has done this. :-)

Log
If applicable, add serial log output to support the analysis.

.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c: In member function 'uint8_t TFT_eSPI::readByte()':
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:115:16: note: in expansion of macro 'TFT_D0'
b = (((reg>>TFT_D0)&1) << 0);
^~~~~~
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:116:16: note: in expansion of macro 'TFT_D1'
b |= (((reg>>TFT_D1)&1) << 1);
^~~~~~
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:117:16: note: in expansion of macro 'TFT_D2'
b |= (((reg>>TFT_D2)&1) << 2);
^~~~~~
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:118:16: note: in expansion of macro 'TFT_D3'
b |= (((reg>>TFT_D3)&1) << 3);
^~~~~~
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:119:16: note: in expansion of macro 'TFT_D4'
b |= (((reg>>TFT_D4)&1) << 4);
^~~~~~
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:120:16: note: in expansion of macro 'TFT_D5'
b |= (((reg>>TFT_D5)&1) << 5);
^~~~~~
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:121:16: note: in expansion of macro 'TFT_D6'
b |= (((reg>>TFT_D6)&1) << 6);
^~~~~~
: warning: right shift count >= width of type [-Wshift-count-overflow]
.pio/libdeps/lilygo-t-display-s3/TFT_eSPI/Processors/TFT_eSPI_ESP32_S3.c:122:16: note: in expansion of macro 'TFT_D7'
b |= (((reg>>TFT_D7)&1) << 7);
^~~~~~

[BlueAndi]

I saw this as well and the issue itself should be fixed in the TFT_eSPI repository. Pixelix at the moment just uses the fork of this lib, provided by HajuSchulz.
@HajuSchulz Can you take care?

[nhjschulz]

I'll check the current state of TFT_eSPI regarding this over the weekend. In early S3 days only this somewhat hasty fork existed that I pinned.

[robertlipe]

FWIW, I work with a lot of chips (team RISC-V) and there are now LOTS of SoCs out there with gpios above their word size (32/64). I've fixed a lot of code in libs and OSes for dealing this this. You can usually find a formula that. Extract these bits into an array index to hit the right regurster and those bits into a bit number. Ch32v307 is like a $2 chip with 80(!) Gpios. It's single core, but otherwise a superset of esp32s3 as it's a newer part. It's just a reality that we embedded types need to be prepared to deal with these days.

…

On Thu, Jul 27, 2023, 1:37 PM Norbert Schulz ***@***.***> wrote: I'll check the current state of TFT_eSPI regarding this over the weekend. In early S3 days only this somewhat hasty fork existed that I pinned. — Reply to this email directly, view it on GitHub <#139 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCSD33C6TK2L2GKLCOWNRLXSKYPPANCNFSM6AAAAAA2X653QA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[nhjschulz]

The GPIO access problem got fixed meanwhile in the official TFT_eSPI repository. I'll prepare a push request to replace the early pinned S3 support patched one with it.

[robertlipe]

Thanx, but it's probably a distraction for me personally. I don't NEED GPIOs > 32. I just noticed the warning and helped analyze it. Let's focus on the partitioning/boot issues, I've admittedly starved it for attention and need to come back to it.

…

On Sun, Jul 30, 2023 at 6:42 AM Norbert Schulz ***@***.***> wrote: The GPIO access problem got fixed meanwhile in the official TFT_eSPI repository. I'll prepare a push request to replace the early pinned S3 support patched one with it. — Reply to this email directly, view it on GitHub <#139 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCSD3ZFTPPFISK5AI7FGZLXSZCDXANCNFSM6AAAAAA2X653QA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BlueAndi]

Thanks Robert and Norbert! :-)