LCD: Merge files for ST7735 and ST7789 into ST77XX #4506
Conversation
Current files for controllers ST7735 and ST7789 are almost identical with only notable diferences to be in pre-set resolution. This commit merges those two files into one called ST77XX. This can be helpful to avoid having diverging functionalities and bugfixes and also opens the doors for easier implementation of other similar controllers (like ST7796 etc.). The configuration options stays separately for ST7735 and ST7789 to make it easier for user to configured those displays and controllers. This also allows to have preset resolutions for different controllers from ST77XX family. Signed-off-by: Michal Lenc <[email protected]>
This commit presents the changes made in 9343474 to board level section. It basically just renames files to st77xx.c, change few options in defconfigs and change macros from ST7735 or ST7789 to ST77XX Signed-off-by: Michal Lenc <[email protected]>
| @@ -619,15 +625,15 @@ if LCD_ST7789 | |||
| ---help--- | |||
| Specifies the Y offset of the LCD. | |||
|
|
|||
| config LCD_ST7789_BPP | |||
| config LCD_ST77XX_BPP | |||
There was a problem hiding this comment.
I'm concerned about having two config options with the same identifier.
Have you tested enabling both LCD_ST7735 and LCD_ST7789?
This is a valid scenario and I believe will probably result in error.
There was a problem hiding this comment.
Hi @gustavonihei I´ve just tried enabling both ST7735 and ST7789 and it configured and compiled NuttX without any problems. But when I run the fb example application it took the resolution of ST7735 instead of ST7789 because of those #ifdefs https://github.com/michallenc/incubator-nuttx/blob/st77xx/drivers/lcd/st77xx.c#L97.
I think the changes I did would not allow to use both 7735 and 7789 at the same time (because of resolution and also some other stuffs that are under ifdefs). It is a question whether we want to keep that option of using both controllers (I don´t know how much are two different displays used, haven´t seen much of those options myself) or to keep those controllers under one file.
There was a problem hiding this comment.
@michallenc maybe you can define the CONFIG_LCD_ST77XX as main option and after enabling it the user can select (using choice) his model as ST7735 or ST7789
There was a problem hiding this comment.
@michallenc maybe you can define the CONFIG_LCD_ST77XX as main option and after enabling it the user can select (using choice) his model as ST7735 or ST7789
Hi @acassis, sorry for the late reply. I´ve already started with those changes you proposed, haven´t pushed them to GIT yet. It still does not solve the problem that it would not allow the user to use both ST7735 and ST7789 (or the driver mentiopned by @PeterBee97 below) drivers together (although I don´t think more than one display is usually used by the user, so it might not be such a problem).
|
Hi Michal, FYI I just opened a PR for adding GC9A01 driver, which is almost identical to ST7789 driver except for some init magic commands and different default orientation configs. You might want to take it into consideration when unifying the drivers :) |
|
Yes, maybe the way to solve it is passing some parameter in the LCD initialize to indicate the type of LCD, this way we could have more than 1 initialization with the same or different type |
Yes, something like in arch/.../imxrt_flexcan.c. I am not sure how this will turn out, It´s likely that I will put this on ice as my semester starts next week and I need to start working on my semestral & bechalor project (also with NuttX btw) and this should be done correctly and carefully otherwise we could end up with 3 or 4 not working lcd drivers. I will see how much time will I be able to put in this merge. |
|
ST has different variants, so please include ST7796 / ST7796UI as well seen with 3.5" 480x320. |
Summary
This PR presents the changes regarding ST7735 and ST7789 controllers discussed last month in NuttX mailing list. It basically merges those two controllers into one single file called ST77XX. Both files for ST7735 and ST7789 were mostly the same except
for different preset resolution and inversion color command in ST7789 driver. This can be helpful to avoid having diverging functionalities and bugfixes and also opens the doors for easier implementation of other similar controllers (like ST7796 etc.)
The configuration options stays separately for ST7735 and ST7789 to make it easier for user to configured those displays and controllers. This also allows to have preset resolutions for different controllers from ST77XX family.
Impact
Boards using ST77XX controllers.
Testing
Tested for ST7789 controller with Teensy 4.1 board, other configurations succesfully configured and compiled.