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

Emulated DOGM using standard HAL TFT and XPT IO #19017

Merged
merged 28 commits into from
Aug 21, 2020

Conversation

rhapsodyv
Copy link
Member

@rhapsodyv rhapsodyv commented Aug 13, 2020

Description

Marlin STMF1 had: 3 FSCM code, 3 TFT SPI code. All seems copy of each other. For example: all the FSCM code had the same bug that made it slow.

The objective is that all UI use a standard set os tft io classes that are available in the HAL. This PR is a continuation work of #18974.

Now we are making emulated DOGM to use the same HAL TFT IO, in

#include HAL_PATH(../../HAL, tft/xpt2046.h) 
#include HAL_PATH(../../HAL, tft/tft_spi.h)
#include HAL_PATH(../../HAL, tft/tft_fsmc.h)

I changed all DOGM _FSMC_ and _SPI_ defines to only _TFT_. Removed old FSCM and SPI TFT codes, in favor of the TFT_IO classes.

Benefits

  • less repeated code
  • more performance (HAL TFT SPI uses 16 bit DMA, the old LVGL SPI DMA were 8 bit, so it became very fast now)
  • less errors
  • more easily to port emulated DOGM to others HAL
  • this makes emulated DOGM available in STM32, as it already have those TFT IO classes!

Related Issues

#18974

TODO List

  • Make emulated DOGM use TFT IO classes from HAL
  • Make emulated DOGM use XPT IO classe from HAL
  • Remove SPI_TFT_* defines
  • unify HAS_FSMC_TFT / HAS_SPI_TFT and HAS_GRAPHICAL_TFT
  • Organize TFT definitions on Chitu V5/V6, Mks Robin Nano V1 and V2 boards.
  • Review all LCD_* and TFT_* defines, as the old copies were creating its own defines, but now we can have only one set

Next PR:

  • Merge LCD init codes from various places (maybe better in a new PR...)
  • TouchButtons offsets linked to DOGM drawing, to removed hardcoded offsets and solve the touch resolution for dogm
  • Now that we have only one xpt class, we can have a generic touch calibration screen that works in dogm and lvgl too.

@rhapsodyv
Copy link
Member Author

@thinkyhead I played a bit with the tft defines. I have organised some boards TFT definitions too. Take a look in the ongoing work.

Now I will focus to make Emulated DOGM use the same XPT class of the others UI, so we will have only one XPT code per Hal!!

@rhapsodyv
Copy link
Member Author

@thinkyhead what do you think about divide the folder Marlin/src/lcd/tft in two parts:

  • Marlin/src/lcd/tft/color_ui -> code specific for the color UI
  • Marlin/src/lcd/tft/ -> init codes, touch logic, etc, things that could be used by other TFT UI. This classes are generic because they use the TFT IO from current HAL.

This way, we could have a class responsible for tft init, that color ui, dogm and lvgl could use. It will allow all the touch logic (hold, double touch, calibration screen) available for all UI too.

@rhapsodyv
Copy link
Member Author

rhapsodyv commented Aug 14, 2020

Or, maybe we need reorganise some folders. In TFT, we have:

  1. TFT IO in each HAL
  2. TFT base code (init, touch, etc)
  3. Standard Marlin UIs (that are supported by more than one board)
  4. Hardware specific UIs (that works only in one hardware/brand)
  5. Serial TFTs (not rendered by marlin)

@rhapsodyv rhapsodyv force-pushed the dogm-use-standard-io branch from eba6682 to 146f412 Compare August 14, 2020 16:00
@rhapsodyv
Copy link
Member Author

TouchButtons could be merged with tft/touch, as the last one have all that needed by the first, and more.

@rhapsodyv
Copy link
Member Author

There's only one xpt now. We can call back the dogm defines of TOUCH_BUTTONS.

@rhapsodyv rhapsodyv changed the title Emulated DOGM using standard HAL TFT IO (SPI and FSMC) Emulated DOGM using standard HAL TFT and XPT IO Aug 14, 2020
@rhapsodyv
Copy link
Member Author

We can change all LCD_FULL_PIXEL_WIDTH/HEIGHT* to just TFT_WIDTH/HEIGHT.
What is TFT, use TFT_ defines.
And LCD is just for non tft devices.

@rhapsodyv
Copy link
Member Author

@thinkyhead do the last commit makes sense for you?

@rhapsodyv
Copy link
Member Author

rhapsodyv commented Aug 15, 2020

One thing that could be changed, is the way we enable the new color ui.

//#define TFT_320x240
//#define TFT_320x240_SPI
// #define TFT_480x320
//#define TFT_480x320_SPI

those defines are too generic. Looks like you just are defining the size and interface of the tft.

It a little weird to have a "TFT_320x240", and defines like TFT_WIDTH and TFT_HEIGHT

Anyway, FSMC_GRAPHICAL_TFT isn't so good either...

@rhapsodyv
Copy link
Member Author

Now that all standard ui share the same base code, we could have something like this:

TFT_INTERFACE SPI|FSMC
TFT_WIDTH 
TFT_HEIGHT
TFT_DRIVER AUTO|ILI9344,etc
TFT_TYPE CLASSIC_UI | COLOR_UI | LVGL_UI

One set of configuration for all standard TFT UIs

@rhapsodyv rhapsodyv force-pushed the dogm-use-standard-io branch from ab865df to cff285c Compare August 15, 2020 21:43
@thinkyhead
Copy link
Member

we could have something like this … One set of configuration for all standard TFT UIs

Those items should be defined in Conditionals_LCD.h according to the specific LCD model selected in Configuration.h. The user should not have to define TFT_WIDTH/HEIGHT in their configuration, since this can always be determined from the selected LCD model.

The emphasis should be on keeping the number of user-facing options minimal, and only things which might be tuned according to preference.

@rhapsodyv
Copy link
Member Author

Those items should be defined in Conditionals_LCD.h according to the specific LCD model selected in Configuration.h. The user should not have to define TFT_WIDTH/HEIGHT in their configuration, since this can always be determined from the selected LCD model.

Right now, it's not really true. Emulated dogm and the new color ui are kind of "generic".
Currently, the user define LCD_FULL_PIXEL_* in their config.
The only exception that could be defined in the pins are fsmc, that the user must never change it.
But only chitu have fixed configs. And nano boards now.
But no other board have it.

Even if it was true, the new Color ui expose it to the user anyway.

The emphasis should be on keeping the number of user-facing options minimal, and only things which might be tuned according to preference.

Sure, but we aren't talking about a specific TFT. Those 3 UI is now kind of generic UI that may works in any TFT brand! That is the point of reflecting it in the config.

@rhapsodyv
Copy link
Member Author

We really have now 3 UI that may work in almost any TFT. The user just need configure the TFT parameters: resolution, interface and driver. And which one of the 3 he wants.

@rhapsodyv
Copy link
Member Author

Today we have 9 configurations!!

// For new color UI
//
//#define TFT_320x240
//#define TFT_320x240_SPI
//#define TFT_480x320
//#define TFT_480x320_SPI
//#define TFT_DRIVER AUTO

// Upscaled 128x64 Marlin UI
//
//#define SPI_GRAPHICAL_TFT
//#define FSMC_GRAPHICAL_TFT

// TFT LVGL UI
//
//#define TFT_LVGL_UI_FSMC  // Robin nano v1.2 uses FSMC
//#define TFT_LVGL_UI_SPI   // Robin nano v2.0 uses SPI

It could be just:

TFT_INTERFACE SPI|FSMC
TFT_WIDTH 
TFT_HEIGHT
TFT_DRIVER AUTO|ILI9344,etc
TFT_TYPE CLASSIC_UI | COLOR_UI | LVGL_UI

Or:

TFT_INTERFACE SPI|FSMC
TFT_480x320
TFT_320x240
TFT_DRIVER AUTO|ILI9344,etc
TFT_TYPE CLASSIC_UI | COLOR_UI | LVGL_UI

@rhapsodyv
Copy link
Member Author

Anyway, right now, we have not merged the duplicated lcd init codes... when we do it, the config could be simplified as above.

@thinkyhead
Copy link
Member

when we do it, the config could be simplified as above.

As long as the updated configuration options are finally settled and stable…. I'm getting tired of updating the Configurations repository because of all the indecision.

@rhapsodyv
Copy link
Member Author

when we do it, the config could be simplified as above.

As long as the updated configuration options are finally settled and stable…. I'm getting tired of updating the Configurations repository because of all the indecision.

When we merge the init code, we get back this subject.
Right now, it's better keep as it is.

@rhapsodyv
Copy link
Member Author

@thinkyhead I think I finished the macro changes (by now). All LCD macros now is for non touch, and all touch macros are TFT_* ones.
I think this PR can be merged, so I will start working in merging all those repeated init codes. In the end of the day, we will have no duplicated code and a wide range of tft support for all 3 UIs 🕺

@rhapsodyv
Copy link
Member Author

I will some last tests before the merge.

@rhapsodyv rhapsodyv marked this pull request as draft August 17, 2020 15:40
@rhapsodyv rhapsodyv marked this pull request as ready for review August 17, 2020 16:20
@rhapsodyv
Copy link
Member Author

rhapsodyv commented Aug 17, 2020

Tests:
Robin Nano V2 + TFT 35 (SPI)

  • Classic UI
  • Color UI
  • LVGL UI

Chitu V5 + TFT 35 (FSMC)

  • Classic UI
  • Color UI
  • LVGL UI

Robin Nano V1.2 + TFT 35 (FSMC)

  • Classic UI
  • Color UI
  • LVGL UI

@thisiskeithb Can you test nano v1.2, if you dont mind and have some time?

Mks tested it, but they said that nano v1.2 needs a config to use hw spi for touch. I changed it, but I can't test. It would good if you can do a double check.

@thisiskeithb
Copy link
Member

It'd be helpful to know which options I should/can choose for a MKS Robin Nano 1.2 and 2.0 with a TFT35 since I can't keep track of all these changes.

Adding comments above each section as to which options apply to which boards/TFTs would be REALLY helpful.

Do you have a cheat sheet?

@rhapsodyv
Copy link
Member Author

It'd be helpful to know which options I should/can choose for a MKS Robin Nano 1.2 and 2.0 with a TFT35 since I can't keep track of all these changes.

Adding comments above each section as to which options apply to which boards/TFTs would be REALLY helpful.

Do you have a cheat sheet?

This PR don't change the current configuration options. I'm planning change it after the last refactoring (the lcd init codes one).

Right now:

  • Emulated DOGM
    • FSMC = FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
    • SPI = SPI_GRAPHICAL_TFT + TOUCH_SCREEN
  • LVGL
    • FSMC = TFT_LVGL_UI_FSMC
    • SPI = TFT_LVGL_UI_SPI
  • Color UI
    • FSMC = TFT_480x320 / TFT_320x240
    • SPI = TFT_480x320_SPI / TFT_320x240_SPI

@thisiskeithb
Copy link
Member

I still think adding some specific board names/TFT models in the comments above each section or somehow group/simplify those options would be helpful to everyone.

I try to follow new PRs and these new UI options, but it's confusing as to which options work for which boards/TFTs and certainly isn't clear for someone looking at this for the first time.

@thinkyhead
Copy link
Member

I concur that it is a lot to track, and lots of new screens are coming out at a rapid pace.

I suggest that we add a page to the Documentation with a photograph of each of these displays along with a list of stats, and that we link to URLs with anchors on that page in the configuration comments.

@rhapsodyv
Copy link
Member Author

These are the boards that have some setup for a Marlin controlled TFT:

TFT_LVGL_UI_(SPI|FSMC) = LVGL UI
(SPI|FSMC)_GRAPHICAL_TFT = Classic UI (emulated dogm)
TFT_(480x320|320x240)(SPI?) = Color UI

  • BOARD_CHITU3D_V5 - FSMC - Stock TFT

    • TFT_LVGL_UI_FSMC
    • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
    • TFT_480x320 + TOUCH_SCREEN
  • BOARD_CHITU3D_V6 - FSMC - Stock TFT

    • TFT_LVGL_UI_FSMC
    • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
    • TFT_480x320 + TOUCH_SCREEN
  • BOARD_LONGER3D_LK - FSMC - Stock TFT?

    • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
    • Can support too:
      • TFT_320x240 + TOUCH_SCREEN
  • BOARD_MKS_ROBIN_MINI - FSMC

    • TFT_320x240 + TOUCH_SCREEN => MKS TFT 28/32??
    • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
    • Can support too:
      • TFT_480x320 + TOUCH_SCREEN => tft 35??
  • BOARD_MKS_ROBIN_PRO - FSMC

    • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
    • Can support too:
      • TFT_320x240 + TOUCH_SCREEN => MKS TFT 28/32??
      • TFT_480x320 + TOUCH_SCREEN => tft 35??
      • TFT_LVGL_UI_FSMC if it have SPI FLASH...
  • BOARD_MKS_ROBIN - FSMC

    • TFT_480x320 + TOUCH_SCREEN
    • TFT_320x240 + TOUCH_SCREEN
    • Can support too:
      • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
      • TFT_LVGL_UI_FSMC if it have SPI FLASH...
  • BOARD_MKS_ROBIN_NANO (v1.2) - FSMC - MKS TFT 35

    • TFT_LVGL_UI_SPI
    • SPI_GRAPHICAL_TFT + TOUCH_SCREEN
    • TFT_480x320_SPI + TOUCH_SCREEN
  • BOARD_MKS_ROBIN_NANO_V2 - SPI - MKS TFT 35

    • TFT_LVGL_UI_SPI
    • SPI_GRAPHICAL_TFT + TOUCH_SCREEN
    • TFT_480x320_SPI + TOUCH_SCREEN
    • Can support too:
      • TFT_320x240 + TOUCH_SCREEN = mks tft 28/32
  • BOARD_TRIGORILLA_PRO - FSMC - Stock TFT?

    • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
    • Can support too:
      • TFT_320x240 + TOUCH_SCREEN => MKS TFT 28/32??
      • TFT_480x320 + TOUCH_SCREEN => tft 35??
      • TFT_LVGL_UI_FSMC if it have SPI FLASH...
  • BOARD_LERDGE_K - FSMC

    • TFT_480x320 + TOUCH_SCREEN
    • TFT_320x240 + TOUCH_SCREEN
    • Can support too:
      • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
      • TFT_LVGL_UI_FSMC if it have SPI FLASH...
  • BOARD_LERDGE_S - FSMC

    • TFT_480x320 + TOUCH_SCREEN
    • TFT_320x240 + TOUCH_SCREEN
    • Can support too:
      • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
      • TFT_LVGL_UI_FSMC if it have SPI FLASH...
  • BOARD_LERDGE_X - FSMC

    • TFT_480x320 + TOUCH_SCREEN
    • TFT_320x240 + TOUCH_SCREEN
    • Can support too:
      • FSMC_GRAPHICAL_TFT + TOUCH_SCREEN
      • TFT_LVGL_UI_FSMC if it have SPI FLASH...

@rhapsodyv
Copy link
Member Author

From that list, I refactored: BOARD_CHITU3D_V5, BOARD_CHITU3D_V6, BOARD_MKS_ROBIN_NANO and BOARD_MKS_ROBIN_NANO_V2 to support everything that I or @thisiskeithb could test.

@rhapsodyv
Copy link
Member Author

I suggest that we add a page to the Documentation with a photograph of each of these displays along with a list of stats, and that we link to URLs with anchors on that page in the configuration comments.

It would help a lot. But remember: this 3 UI are generic, they aren't brand specific. The board just need have pins for TOUCH, and FSMC or the SPI. And done. The TFT will work. The user may need some further setup, like rotating it or discovering the init code that works. But we need to keep mind is that: those 3 ui aren't anymore board or brand specific.

@rhapsodyv
Copy link
Member Author

rhapsodyv commented Aug 21, 2020

That is the reason that I will suggest (in the next PR) to change all those confusing TFT configs to something like:

TFT_INTERFACE SPI|FSMC
TFT_480x320
TFT_320x240
TFT_DRIVER AUTO|ILI9344,etc
TFT_ROTATE_180 
TFT_ROTATE_90
TFT_MIRROR_VERTICAL
TFT_MIRROR_HORIZONTAL
TFT_TYPE CLASSIC_UI | COLOR_UI | LVGL_UI

@thinkyhead
Copy link
Member

thinkyhead commented Aug 21, 2020

TFT_ROTATE_180 
TFT_ROTATE_90

For a long time Marlin has had these settings (for MINIPANEL)…

//#define LCD_SCREEN_ROT_90
#define LCD_SCREEN_ROT_180
//#define LCD_SCREEN_ROT_270

…and I suggest that all the screen rotation settings be consolidated down to these (or better yet, a single setting taking a value of 0, 180, or 270). So that would include the settings above plus:

  • GRAPHICAL_TFT_ROTATE_180
  • ST7735_ORIENTATION (set value based on rotation)
  • ST7789V_ORIENTATION
  • ST7796S_ORIENTATION
  • ILI9341_ORIENTATION
  • ILI9488_ORIENTATION
  • TOUCH_ORIENTATION

@rhapsodyv
Copy link
Member Author

//#define LCD_SCREEN_ROT_90
#define LCD_SCREEN_ROT_180
//#define LCD_SCREEN_ROT_270

It is for text/graphical LCD, right? I think we can have a separated set of configs, LCD_ ones for non TFT, and TFT_ ones for TFT displays.

…and I suggest that all the screen rotation settings be consolidated down to these (or better yet, a single setting taking a value of 0, 180, or 270). So that would include the settings above plus:

Sure, the general idea is unify all those TFT options. But it could only be done after we join and refactor all those lcd init code.
Right now we have 3 sets of lcd init codes... one for lvgl, one for color ui and one for Emulated DOGM.......

The user will show the rotation and other params, and all those init code will be ready to accept it. Right now, only a few init codes are ready for those options.

I will start this work after this PR get merged.

@thinkyhead thinkyhead merged commit a37cf24 into MarlinFirmware:bugfix-2.0.x Aug 21, 2020
@thinkyhead
Copy link
Member

🐕🛷!

albertogg pushed a commit to albertogg/Marlin that referenced this pull request Aug 31, 2020
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request Sep 2, 2020
@rhapsodyv rhapsodyv deleted the dogm-use-standard-io branch October 25, 2020 00:33
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request Dec 9, 2020
kageurufu pushed a commit to CR30-Users/Marlin-CR30 that referenced this pull request Apr 30, 2021
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