Skip to content

Fix SPI TFT Compile Errors on SKR V3/EZ & V2 - #26041

Merged
thinkyhead merged 4 commits into
MarlinFirmware:bugfix-2.1.xfrom
thisiskeithb:pr/fix_spi_tft_skr_v3
Jul 17, 2023
Merged

Fix SPI TFT Compile Errors on SKR V3/EZ & V2#26041
thinkyhead merged 4 commits into
MarlinFirmware:bugfix-2.1.xfrom
thisiskeithb:pr/fix_spi_tft_skr_v3

Conversation

@thisiskeithb

@thisiskeithb thisiskeithb commented Jul 2, 2023

Copy link
Copy Markdown
Contributor

Description

Followup to #25784 since enabling an SPI TFT on an SKR 3/EZ & V2 would throw several errors about variables not being declared:

error: 'TFT_SCK_PIN' was not declared in this scope; did you mean 'TFT_DC_PIN'?
error: 'TFT_MOSI_PIN' was not declared in this scope; did you mean 'SD_MOSI_PIN'?
error: 'TFT_MISO_PIN' was not declared in this scope; did you mean 'SD_MISO_PIN'?
error: 'TFT_SCK_PIN' was not declared in this scope; did you mean 'TFT_DC_PIN'?

This reverts later changes in the original PR to be more like what I had originally.

Verified on an SKR V3.0 EZ with a BTT TFT35-SPI V1.0 and MKS TS35 V2.0

Requirements

SKR V3/EZ with a BTT TFT35-SPI V1.0 or MKS TS35 V2.0

Benefits

Config will compile

Configurations

#define MOTHERBOARD BOARD_BTT_SKR_V3_0 or #define MOTHERBOARD BOARD_BTT_SKR_V3_0_EZ with: #define MKS_TS35_V2_0 or #define BTT_TFT35_SPI_V1_0.

Related Issues

@thisiskeithb

Copy link
Copy Markdown
Contributor Author

BTT_TFT35_SPI_V1_0 no longer works on an SKR 2 (so the B1 SE / B1 SE Plus builds are broken), but I'm still looking into it/testing some changes. I can only get a white screen when adding the missing pins back in.

@thisiskeithb

thisiskeithb commented Jul 2, 2023

Copy link
Copy Markdown
Contributor Author

Even fully reverting pins_BTT_SKR_V2_0_common.h to the state it was before #25784, I still get a white screen with BOARD_BTT_SKR_V2_0_REV_B & BTT_TFT35_SPI_V1_0.

If I drop the 3ea87ad / #25784 commit completely, this combination works fine.

After more testing & trying out the original commit in #25784 (before everything was rebased / updated), I still get a white screen with a BTT_TFT35_SPI_V1_0 on BOARD_BTT_SKR_V2_0_REV_B (STM32F4).

Seems that SPI TFT support on STM32F4 was broken while updating for STM32H7.

@thisiskeithb thisiskeithb changed the title Fix SPI TFT Compile Errors on SKR V3 Fix SPI TFT Compile Errors on SKR V3/EZ Jul 2, 2023
@thinkyhead

Copy link
Copy Markdown
Member

Seems that SPI TFT support on STM32F4 was broken while updating for STM32H7.

We'll need to confer with @jmz52 about those changes and see what still needs patching there.

@thinkyhead

Copy link
Copy Markdown
Member

These pins are similarly configured in pins_BTT_SKR_V1_3.h so maybe that file also needs a look.

@thisiskeithb

thisiskeithb commented Jul 4, 2023

Copy link
Copy Markdown
Contributor Author

These pins are similarly configured in pins_BTT_SKR_V1_3.h so maybe that file also needs a look.

From my testing, SPI TFTs have never worked on LPC, so they should probably be removed.

We'll need to confer with @jmz52 about those changes and see what still needs patching there.

I also pinged them in #25784

@jmz52

jmz52 commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

I'll look into it, but I can't test H7 code right now, my H7 board is out of the reach.
In the mean time, can you please check if forcing TFT driver still results in white screen
#define TFT_DRIVER ST7796
This will bypass TFT driver detection code, so it might help to pinpoint the problem.

@thisiskeithb

Copy link
Copy Markdown
Contributor Author

I'll look into it, but I can't test H7 code right now, my H7 board is out of the reach.

SPI TFT's work on STM32H7. They're broken on STM32F4 now.

@thisiskeithb
thisiskeithb force-pushed the pr/fix_spi_tft_skr_v3 branch from d6c36d6 to 427040b Compare July 4, 2023 21:52
@thisiskeithb

Copy link
Copy Markdown
Contributor Author

427040b fixes the SKR 2 pins similar to how I fixed the SKR V3 pins. This still doesn't work, but it'll compile now.

@thinkyhead

thinkyhead commented Jul 4, 2023

Copy link
Copy Markdown
Member

SPI TFT's work on STM32H7. They're broken on STM32F4 now.

The changes in #25784 affecting F4 are very few. You can try reverting just those few things (particularly in HAL/STM32/tft/tft_spi.cpp) to see if any of those small changes help. For example, this line was removed…

pin_PullConfig(get_GPIO_Port(STM_PORT(digitalPinToPinName(TFT_SCK_PIN))), STM_LL_GPIO_PIN(digitalPinToPinName(TFT_SCK_PIN)), GPIO_PULLDOWN);

And also this was removed (although it is called as part of dataTransferBegin):

HAL_SPI_Init(&SPIx);

Even if these were intentional removals, maybe there is something about them that helps with F4.

@thisiskeithb

thisiskeithb commented Jul 4, 2023

Copy link
Copy Markdown
Contributor Author

As for SPI TFT support on LPC:

I went allllll the way back to #21794 and tested the PR as-is again. MKS' TS35 V2.0 does not work on the SKR 1.3, so those changes can be dropped. I remember trying to get both BTT's and MKS' SPI TFTs working on LPC before and couldn't, so I'm not sure they ever worked.

We still have HAS_SPI_TFT in several other pins files that need to be updated as well since that is not universal for both BTT's and MKS' SPI TFTs.

@thisiskeithb

Copy link
Copy Markdown
Contributor Author

CI is still failing due to 1792608.

@thisiskeithb thisiskeithb changed the title Fix SPI TFT Compile Errors on SKR V3/EZ Fix SPI TFT Compile Errors on SKR V3/EZ & V2 Jul 4, 2023
@thinkyhead
thinkyhead force-pushed the pr/fix_spi_tft_skr_v3 branch from b9d961f to 0051698 Compare July 4, 2023 23:45
thisiskeithb and others added 3 commits July 5, 2023 08:03
Verified on an SKR V3.0 EZ with a BTT TFT35-SPI V1.0 and MKS TS35 V2.0
TFT supports on STM32F4 is still broken, but this will at least compile.
@thisiskeithb

thisiskeithb commented Jul 5, 2023

Copy link
Copy Markdown
Contributor Author

I cherry picked changes from #26052 into this PR 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.

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

See #26052 (comment) for the fix and more info.

There are still some major glitches going on, so this still isn't entirely fixed.

@thisiskeithb
thisiskeithb force-pushed the pr/fix_spi_tft_skr_v3 branch from b083d8c to 557fa70 Compare July 5, 2023 16:10
@thisiskeithb
thisiskeithb force-pushed the pr/fix_spi_tft_skr_v3 branch from 557fa70 to 9e2fddd Compare July 5, 2023 16:12
@thinkyhead

Copy link
Copy Markdown
Member

There are still some major glitches going on, so this still isn't entirely fixed.

Did we narrow down the definite culprit in this issue? Does reverting the STM32H7 stuff fix everything, or is some other set of changes involved? I made a number of TFT case changes that were not intended to change any behavior, but there could still be a typo hiding in there.

@thisiskeithb

thisiskeithb commented Jul 6, 2023

Copy link
Copy Markdown
Contributor Author

Did we narrow down the definite culprit in this issue?

Nope.

Does reverting the STM32H7 stuff fix everything

Yep. If I roll back to the commit prior (c483fe7) to the SPI TFT on STM32H7 (3ea87ad) commit, all these issues go away for STM32F4 (at least on the B1 SE Plus / SKR2 / BTT TFT35 SPI).

If this can’t easily be fixed, I think doing a full revert on these recent SPI TFT on H7 commits and combining them in #26052 (or a new PR) for more testing would be better than breaking existing support on F4.

@thinkyhead
thinkyhead merged commit fef772e into MarlinFirmware:bugfix-2.1.x Jul 17, 2023
@thisiskeithb
thisiskeithb deleted the pr/fix_spi_tft_skr_v3 branch July 17, 2023 05:15
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 17, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 20, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
vovodroid added a commit to vovodroid/Marlin that referenced this pull request Sep 14, 2023
vovodroid added a commit to vovodroid/Marlin that referenced this pull request Oct 13, 2023
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request Oct 26, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
vovodroid added a commit to vovodroid/Marlin that referenced this pull request Nov 10, 2023
vovodroid added a commit to vovodroid/Marlin that referenced this pull request Nov 14, 2023
vovodroid added a commit to vovodroid/Marlin that referenced this pull request Nov 14, 2023
vovodroid added a commit to vovodroid/Marlin that referenced this pull request Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants