Fix SPI TFT for STM32F1/F4

[jmz52]

Description

Fix SPI TFT for STM32F4 issue introduced in #25784

Also fixed an issue with macro pins check for SPI pins when pins are defined as reference to analog pins instead of numbers

variant.h:
#define PA7  A7

Requirements

STM32F4 board and TFT display with SPI interface

Benefits

STM32F4 boards (SKR v2 and MKS Robin Nano v3.x) can again be used with SPI TFT displays.

Configurations

Related Issues

[thisiskeithb]

I cherry picked these changes into #26041 and the BOARD_BTT_SKR_V2_0_REV_B + MKS_TS35_V2_0 combination works, but BOARD_BTT_SKR_V2_0_REV_B + BTT_TFT35_SPI_V1_0 does not.

I'll do some more testing today to see if it's an issue of incorrectly defined pins or something is still broken.

[thisiskeithb]

BTT_TFT35_SPI_V1_0 pinout seems fine. Something is still broken for this screen on STM32F4.

[jmz52]

@thisiskeithb, MKS_TS35_V2_0 and BTT_TFT35_SPI_V1_0 have the same ST7796 TFT driver.
If MKS_TS35_V2_0 work with BOARD_BTT_SKR_V2_0_REV_B then BTT_TFT35_SPI_V1_0 should work too.

Lerdge TFT display is known to report custom ID, so forcing exact driver might help to bypass identification part
#define TFT_DRIVER ST7796

Other that this it should be either pins definition, cable orientation issue or forgotten TFT reset pin (if there is one on this display)
TFT initialization code is pretty much back to 'pre H7' state, so at the very least you should see random color pixels on the screen.

[thisiskeithb]

Lerdge TFT display is known to report custom ID, so forcing exact driver might help to bypass identification part
#define TFT_DRIVER ST7796

That was the fix!

...which is interesting since this should be set already:

Marlin/Marlin/src/inc/Conditionals_LCD.h

Lines 1641 to 1642 in 57d26f7

	#if ANY(MKS_TS35_V2_0, BTT_TFT35_SPI_V1_0) // ST7796
	#define TFT_DEFAULT_DRIVER ST7796

edit: I see some GUI glitches in both the BTT_TFT35_SPI_V1_0 & MKS_TS35_V2_0 like red or blue rectangles/blocks that span the full width of the screen and flashes/glitching of various icons, so it's not quite working as it used to:

To confirm it wasn't an issue with the hardware I'm testing on my desk, I also flashed my Biqu B1 SE Plus which has the same BTT_TFT35_SPI_V1_0 TFT & BOARD_BTT_SKR_V2_0_REV_B motherboard and it also has the same issues.

[jmz52]

TFT_DEFAULT_DRIVER is used if the id read from the controller is 0x0000 or 0xFFFF.
If id is reported but unknown then TFT_DEFAULT_DRIVER is ignored and no TFT initialization is performed.

TFT_DRIVER bypass TFT identification completely and force the specific driver.

[thisiskeithb]

TFT_DEFAULT_DRIVER is used if the id read from the controller is 0x0000 or 0xFFFF.
If id is reported but unknown then TFT_DEFAULT_DRIVER is ignored and no TFT initialization is performed.

TFT_DRIVER bypass TFT identification completely and force the specific driver.

Adding #define TFT_DRIVER ST7796 wasn't required before, but should it be set now Conditionals_LCD.h?

[jmz52]

@thisiskeithb, I was not able to reproduce these glitches on my MKS Robin Nano v3.1, so I'll need your help to find the root cause.
Here is my understanding of the current state of the code, please correct me if I am wrong.

• SKR v3 works with both MKS and BTT screens and can autodetect driver on both of them, no visual glitches
• SKR v2 worked on pre-H7 code both MKS and BTT screens and could autodetect driver on both of them, and there were no visual glitches
• SKR v2 can autodetect MKS screen but not the BTT screen on H7 code with this fix applied. visual glitches on both MKS and BTT screens

If this is correct, that the first and easiest to validate step should be fixing autodetection of BTT display. Visual glitches might be caused by the same SPI issues that are failing autodetection. I need you to run autodetection on SKR v3 board and check for ID that is actually reported by the ST7796 controller (to make sure that it does report valid ID and not 0x0000 or 0xFFFF where default drivers steps in)

[thisiskeithb]

I'll need your help to find the root cause

I will retest each of these commits/scenarios this week. Keep in mind that I'll need to apply pin changes from #26041 since #25784 was merged with compile errors for these boards/displays and that HAS_SPI_TFT macro is not universal for these displays due to different pinouts.

I need you to run autodetection on SKR v3 board and check for ID that is actually reported by the ST7796 controller (to make sure that it does report valid ID and not 0x0000 or 0xFFFF where default drivers steps in)

How do I get the ID reported from the ST7796 controller?

[jmz52]

How do I get the ID reported from the ST7796 controller?

You need to add SERIAL_ECHOLN are the end of TFT_SPI::ReadID() function and check console output.
https://github.com/MarlinFirmware/Marlin/blob/2.1.2.1/Marlin/src/HAL/STM32/tft/tft_spi.cpp#L175

[thisiskeithb]

You need to add SERIAL_ECHOLN are the end of TFT_SPI::ReadID() function and check console output.

Adding something as simple as SERIAL_ECHOLN("Testing"); doesn't dump anything to serial, so I'm guessing that it happens too early in the boot process? I'm not sure how to get the TFT ID for you.

• SKR v2 worked on pre-H7 code both MKS and BTT screens and could autodetect driver on both of them, and there were no visual glitches

Correct. Using Marlin as of c483fe7 (right before 3ea87ad was merged) and adding pin changes from #26041, I do not need to add #define TFT_DRIVER ST7796 to my config, so auto detection appears to be working correctly(?) on BOARD_BTT_SKR_V2_0_REV_B and both BTT_TFT35_SPI_V1_0 & MKS_TS35_V2_0 TFTs. There are also no visual glitches like I'm seeing with current bugfix-2.1.x/this PR.

• SKR v2 can autodetect MKS screen but not the BTT screen on H7 code with this fix applied. visual glitches on both MKS and BTT screens

Correct. Note: I had to add pin changes from #26041 as well.

• SKR v3 works with both MKS and BTT screens and can autodetect driver on both of them, no visual glitches

Note quite. BOARD_BTT_SKR_V3_0/BOARD_BTT_SKR_V3_0_EZ with pin changes from #26041, can autodetect both TFTs. Testing with the latest bugfix-2.1.x (bcf61cc) and code from this (#26052) PR. Note: MKS_TS35_V2_0 glitches red/green/blue blocks spanning the entire TFT width during any UI sounds (like initializing EEPROM settings)/M300 and there's random lines of red if you scroll through parts of the UI too fast. BTT_TFT35_SPI_V1_0 seems to be fine now.

Just to make sure it wasn't hardware, I tested on multiple BOARD_BTT_SKR_V3_0/BOARD_BTT_SKR_V3_0_EZ/BOARD_BTT_SKR_V2_0_REV_B boards (including my Biqu B1 SE Plus) as well as multiple BTT_TFT35_SPI_V1_0 & MKS_TS35_V2_0 TFTs. All issues were reproducible across each hardware combos.

[jmz52]

That is a bit unexpected, SERIAL_ECHOLN is what I am using for debugging TFT code.
Are us using onboard USB on external USB/UART converter (like FT232 or CH340) connected to onboard UART?
Are "start" and reset reason messages shown in console?

There is another way to check for ID - disable TFT_DEFAULT_DRIVER for BTT_TFT35_SPI_V1_0 and check if SKR3 still can autodetect the TFT controller. If it does, then the ID reported is exactly the 0x7796 - Marlin has only one ID for this controller in tft_ids.h:
#define ST7796 0x7796

Note: MKS_TS35_V2_0 glitches red/green/blue blocks spanning the entire TFT width during any UI sounds (like initializing EEPROM settings)/M300 and there's random lines of red if you scroll through parts of the UI too fast. BTT_TFT35_SPI_V1_0 seems to be fine now.

Please check if problem with MKS_TS35_V2_0 persist if BEEPER_PIN is removed from pins file. And if the glithec are still present with BEEPER_PIN intact, but with disabled touch screen when everything is controller via encoder.

MKS screen shares HW SPI with touch controller, while BTT uses dedicated pins for touch and software spi to read data from it.
This difference might be the reason why there are no glitches on BTT screen.

[thisiskeithb]

Please check if problem with MKS_TS35_V2_0 persist if BEEPER_PIN is removed from pins file.

Testing this PR on BOARD_BTT_SKR_V2_0_REV_B + MKS_TS35_V2_0 (and of course, pin changes from #26041), glitches go away with the BEEPER_PIN disabled.

And if the glithec are still present with BEEPER_PIN intact, but with disabled touch screen when everything is controller via encoder.

With BEEPER_PIN enabled and TOUCH_SCREEN disabled with BOARD_BTT_SKR_V2_0_REV_B + MKS_TS35_V2_0, glitches are still there and sometimes the UI goes into submenus on its own when sending / playing tones. I also seem to have stumbled upon a new bug. The lower part of the UI is now completely missing on most screens with TOUCH_SCREEN disabled:

I have one MKS_TS35_V2_0 with an encoder and one without, so disabling TOUCH_SCREEN wouldn't make sense for the one without, but the UI should still display correctly either way.

There is another way to check for ID - disable TFT_DEFAULT_DRIVER for BTT_TFT35_SPI_V1_0 and check if SKR3 still can autodetect the TFT controller.

Disabling TFT_DEFAULT_DRIVER for BOARD_BTT_SKR_V3_0_EZ + BTT_TFT35_SPI_V1_0 works. The TFT is auto-detected and seems to work as expected.

Are us using onboard USB on external USB/UART converter (like FT232 or CH340) connected to onboard UART?

Onboard USB.

Are "start" and reset reason messages shown in console?

No.

[jmz52]

I have one MKS_TS35_V2_0 with an encoder and one without, so disabling TOUCH_SCREEN wouldn't make sense for the one without, but the UI should still display correctly either way.

Invisible answer selection (Init EEPROM) was just fixed in #26077

Edit screen ("nozzle") was designed to hide "-" "v" and "+" icons when touch screen is disabled.
These elements can't be interacted with and provide no additional information.
The "slider is intended to be reduced to "progress bar" - no draggable element, but still shows where in range of possible values the current value is.

[phcay]

I have an MKS Robin Nano v3.0 with an MKS TS35V2 w/encoder.

And indeed, in the bugfix-2.1.x branch, from the "SPI TFT / Touchscreen for STM32H7 (#25784)", I have a white screen from the boot. I can control the Robin Nano with USB, but the screen is unusable.
Using the parent commit "Fix ProUI _remain_time" it works again.

Then I found this PR. So I tested your tft_spi.cpp with the latest "[cron] Bump distribution date (2023-07-06)", and for my configuration it works.

Thanks for this work, and also for "Rework TFT Color UI layout #26077"

[thisiskeithb]

I just discovered that I get a solid white screen with MKS_TS35_V2_0 on BOARD_MKS_ROBIN_NANO_V2 (STM32F1-based), so maybe F1 is also affected by similar F4 bugs.

I'd have to do some git bisecting to find out when it was broken though.

[jmz52]

@thisiskeithb, what code you were using when you've got white sreen on MKS Robin Nano v2 - bugfix or code from this patch?

[thisiskeithb]

what code you were using when you've got white sreen on MKS Robin Nano v2 - bugfix or code from this patch?

Current bugfix-2.1.x. I'll do some more testing this weekend and see if F1 is affected the same way F4 is or if this is an entirely new bug.

Speaking of this PR, I just rebased it on current bugfix since several TFT-related PRs have gone in that will make testing easier.

[jmz52]

F1 and F4 shares most of the code, so I would expect the root cause to be the same - TFT_SPI::transmit()
Could you please test this PR on F1 with and without forced TFT_DRIVER?

[phcay]

I also have an MKS Robin Nano v2.0 with an MKS TS35V2 with encoder in a drawer.
So I did some tests with the same sources as I did with the MKS Robin Nano v3.0 (bugfix-2.1.x "[cron] Bump distribution date (2023-07-06)". It's not connected to a printer and it's powered only via USB, but that should be enough to test that.
I did not impose TFT_DRIVER, and I checked that it was also the case in pins_MKS_ROBIN_NANO_common.h and pins_MKS_ROBIN_NANO_V2.h.
Attention, to remove any ambiguity, do not touch the tft_spi.cpp of the STM32F1, only that of the STM32 must be replaced (the same as for the NANO_V3).

NanoV2+TS35V2 without patch tft_spi.cpp: White blank screen
NanoV2+TS35V2 with tft_spi.cpp patch: Screen works
NanoV2+TS35V2 [maple] without patch tft_spi.cpp: Screen works
NanoV2+TS35V2 [maple] with tft_spi.cpp patch: Screen works

[phcay]

To isolate which fixes impact my MKS motherboards, I independently tested the 2 main changes in this PR:

Moved MISO and MOSI pin comparison conditions from preprocessor to compiler.
Restoration of part of the code of the TFT_SPI::transmit(uint16_t data) function for non STM32H7xx.

I checked the pin assignment for the tft_spi, the touchscreen and the encoder button, they are identical between the NanoV2 (STM32F103) and the NanoV3 (STM32F407).

Note that on the MKS_NanoV2 and MKS_NanoV3, the MISO and MOSI pins are of the same type (PA), so switching the preprocessor conditions to the compiler should not change the operation for these 2 cards (apart from a small flash consumption of 16 bytes), but that does not mean that it does not correct for cards where they would be of different type.

TFT_MISO_PIN: PA6
TFT_MOSI_PIN: PA7

NanoV2+TS35V2 [tft_spi.cpp - original]: White blank screen
NanoV2+TS35V2 [tft_spi.cpp - PR26052]: Screen works
NanoV2+TS35V2 [tft_spi.cpp - only condition preproc 2 compiler fix]: White blank screen
NanoV2+TS35V2 [tft_spi.cpp - only TFT_SPI::transmit(uint16_t data) fix]: Screen works

I did not perform the tests on the NanoV3, because it is in use. I can do some tests later, but a priori, it should be the same.

So it is indeed TFT_SPI::transmit(uint16_t data) which corrects the operation for the NanoV2. Indeed, following the implementation in the tft_spi.cpp for the STM32H7xx after 06/30/2023, the resulting code for non STM32H7xx has been changed.
The code of this PR26052 makes it possible to restore the code resulting from this function for non STM32H7xx as it was on 06/30/2023.

functional code:

   __HAL_SPI_ENABLE(&SPIx);
   SPIx.Instance->DR = data;
   while (!__HAL_SPI_GET_FLAG(&SPIx, SPI_FLAG_TXE)) {} // <===
   while ( __HAL_SPI_GET_FLAG(&SPIx, SPI_FLAG_BSY)) {}

instead of this non-working code:

     __HAL_SPI_ENABLE(&SPIx);
     SPIx.Instance->DR = data;
     while (__HAL_SPI_GET_FLAG(&SPIx, SPI_FLAG_BSY)) {}
     __HAL_SPI_DISABLE(&SPIx); // <===

I therefore tested whether it was the absence of the wait for the appearance of the SPI_FLAG_TXE flag or the presence of the __HAL_SPI_DISABLE(&SPIx) which was the problem, so I tested these 2 elements independently.

The SPI_FLAG_TXE present (or not) does not change anything on the NanoV2.
On the other hand, the presence of __HAL_SPI_DISABLE(&SPIx) causes the White blank screen on the NanoV2.

So jmz52's proposal to move __HAL_SPI_DISABLE(&SPIx) only for STM32H7xx instead of being common already seems like a good solution to me. As for waiting for the transmission flag, it may have been set to make other configurations work, but for mine, it has no impact at first glance.

P.S.: sorry if there are strange words, but I don't speak English, it's an auto translation.

[thisiskeithb]

@doshiyo103: I’d recommend using an older version of Marlin or waiting until this PR is merged and issue is fixed in bugfix.

[vovodroid]

@doshiyo103, you can revert commits fef772e and 3ea87ad in bugfix-2.1.x (that's how I run it on B1 SE: SKR2+ SPI TFT)

[doshiyo103]

@thisiskeithb ,Hi,Thank you for your response.
I can wait for this fix,and really thank you did good job for Marlin!

@vovodroid ,Hi,Thank you for your response.
I really want to try the solution you mentioned, but this is my first time using GitHub,
so I'm not quite sure about the meaning of 'revert commits #fef772e and #3ea87ad in bugfix-2.1.x.'
Could you please tell me how to do it?

Thank you very much for your assistance.
I'm happy to hear that this method might allow me to use the firmware!

[vovodroid]

meaning of 'revert commits #fef772e and #3ea87ad in bugfix-2.1.x.'

It means remove changes from branch as if they didn't exist. It's not matter of Github, but Git itself. You should clone Marlin repository to your computer and discard (revert) these commits with Git or any Git-Gui shell (I use Gitextension on Windows).

[doshiyo103]

@vovodroid ,
HI,I think I did it. I downloaded GitExtensions to operate,
successfully reverted the two commits you mentioned, then rebuilt.
Now the startup is normal, and there is no delay in operation.

Thank you very much for your assistance!!!!
However, I still look forward to everything being resolved smoothly, bringing a better experience to everyone.
Thanks to everyone, you're great!!

[thisiskeithb]

DEBUG_TFT_IO is now disabled by default in 2483e3c and I wasn't sure if SPIx.Init.BaudRatePrescaler = BaudRatePrescaler; should stick around, so I made it a conditional of DEBUG_TFT_IO so screens will run at full speed unless DEBUG_TFT_IO is enabled.

@jmz52: This PR is also now up to date with current bugfix, but is there anything else to do? The MKS TS35 issue has existed long before these SPI TFT updates based on testing we've done here.

[firepower9966]

Any Updates ? when PR goes to commit, what the usual time frame?

[thisiskeithb]

when PR goes to commit, what the usual time frame?

PRs get merged when they are ready. This PR took an especially long time since jmz52 doesn't have the exact hardware on hand and had to remote test through me since I have all the hardware.

There's still more work to be done to improve SPI TFT support on STM32F1 & STM32F4, but that'll be in a future PR.

@jmz52: Please let me know if there's additional testing/hardware configs that I should run code on.

[firepower9966]

I can also test I have Mks Nano Robin V3.1 and MKS TS35 V2.0 with MKS TMC2209 uart mode and MKSWifi with ESP3D 2.1.2 firmware. How can i test PR?

[firepower9966]

I manual edited the commits into my bugfix-2.1.x that gave me white screen , and now it test OK.

updated
Test ok with latest bugfix-2.1.x dated 1/1/24

[Conan877]

I am willing to buy MKS Robin Nano V3.1 + MKS TS35 V2 today, but I haven't found it yet. Has this problem been solved?

[thisiskeithb]

Has this problem been solved?

Yes, this PR has been merged.