drivers/lcd: Add ST7796 TFT LCD framebuffer driver#18301
drivers/lcd: Add ST7796 TFT LCD framebuffer driver#18301vrmay23 wants to merge 1 commit intoapache:masterfrom
Conversation
Add support for ST7796 TFT LCD controller (320x480). The driver implements the NuttX framebuffer interface for SPI-connected displays. Features: - SPI interface with CONFIG_SPI_CMDDATA for D/C pin control - RGB565 (16-bit) and RGB666 (18-bit) color formats - Runtime rotation support (0, 90, 180, 270 degrees) via MADCTL - Board-provided configuration via st7796_config_s structure - Partial screen update via updatearea for efficient rendering - Persistent swap buffer to avoid per-frame allocations - Proper ST7796S initialization sequence with documented timing The driver uses a board-provided configuration structure allowing flexible setup of resolution, SPI frequency, color depth, and initial MADCTL value without requiring Kconfig options in the generic driver. Tested with LVGL graphics library on STM32H7 platforms. Signed-off-by: Vinicius May <vmay.sweden@gmail.com>
vrmay23
requested review from
acassis,
raiden00pl and
simbit18
as code owners
github-actions
bot
added
Area: Drivers
| #include <nuttx/signal.h> | ||
| #include <nuttx/lcd/st7796.h> | ||
|
|
||
| #ifdef CONFIG_LCD_ST7796 |
There was a problem hiding this comment.
@vrmay23 you don't need this extra guard because this code only will be compiled when CONFIG_LCD_ST7796 is defined (via Make.defs or CMakeLists.txt)
There was a problem hiding this comment.
it's better to remove the unnecessary guard to simplfy the code base.
There was a problem hiding this comment.
I agree on this one. Thanks for feedback!
There was a problem hiding this comment.
But, just to be fair: we have at line 44:
#ifdef CONFIG_LCD_ST7789
and at line 1049:
#endif /* CONFIG_LCD_ST7789 */
So, I indeed have followed the same proccess. But I agree this is redundancy check.
There was a problem hiding this comment.
@vrmay23 you can submit a fix for the other drivers later if you get motivated to do so.
| * | ||
| ****************************************************************************/ | ||
|
|
||
| FAR struct fb_vtable_s *st7796_fbinitialize(FAR struct spi_dev_s *spi, |
There was a problem hiding this comment.
To be honest, I don't think this is a good design. The controller shouldn't depend on frame buffer code and LCD_FRAMEBUFFER option at all. Register it as lcd_dev_s device and use board_lcd_getdev to provide interface between framebuffer driver and lcd driver.
The approach in this MR is completely different from what others LCD drivers use - take a look at st7789 or some ili controllers for example-
There was a problem hiding this comment.
Thank you, in advance for the feedback. I wanna to explain a little bit better the reason I choosed some stuff. Please, read my comments below:
So, the designed architecture is different for many reasons:
ST7789 (LCD driver /dev/lcd0):
line-by-line rendering via putrun(), getrun(), putarea()
No true framebuffer - only a single line buffer (runbuffer)
Application must call putrun for each modified line
Cannot do "dirty" region tracking
// ST7789 - single line buffer only
uint16_t runbuffer[ST7789_LUT_SIZE]; // ~640 bytes for 320px
Application must call this for EVERY line it modifies:
pinfo->putrun = st7789_putrun; /* Put a run into LCD memory /
pinfo->putarea = st7789_putarea; / Put an area into LCD */
pinfo->buffer = (FAR uint8_t )priv->runbuffer; / Run scratch buffer */
ST7796 (Framebuffer /dev/fb0):
Full framebuffer allocated in RAM --> you know in advanced how many bytes you are gonna to use.
Driver only pushes dirty regions via updatearea()
ioctl(fd, FBIO_UPDATE, &area);
Application writes directly to memory
pinfo->fbmem = priv->fbmem; /* Direct pointer to framebuffer /
pinfo->fblen = fbsize; / Total framebuffer size /
pinfo->stride = priv->config.xres * bytespp; / Bytes per line */
True double-buffering capable architecture --> quite useful for advanced HMI applications
Directly compatible with framebuffer examples and LVGL.
// ST7796 - complete framebuffer
priv->fbmem = kmm_zalloc(fbsize); //480 × 320 = 153,600 pixels. 153,600 × 2 bytes = 307,200 bytes = ~300KB
// ST7796 allocates once during init (if I'm not wrong ST7789 need stack allocation / malloc.
priv->swap_buf = kmm_malloc(config->xres * bytespp);
So, in the end, the controller SHOULD depend on framebuffer code because it's implemented as a framebuffer driver. Forcing it into lcd_dev_s is like forcing a USB driver into a serial port interface - technically possible but architecturally wrong. Being honnest, I think ST7789 one should be refactored towards ST7796. Hence, I think no changes are needed in the driver.
There was a problem hiding this comment.
Yes, but as you pointed out, complete framebuffer can take dozens or hunders of KBs of memory - your example with 480 x 320 resolution and 2 bytes color is 300 KBs. That's just unusable for a lot of embedded devices that NuttX targets. LVGL is bad enough with its own memory management even without the full framebuffer.
But my point was slightly different. ST7789 driver can still be used with the full framebuffer from API point of view. You just enable LCD_FRAMEBUFFER and VIDEO_FB options and initialize the framebuffer from BSP. For instance boards/arm/imxrt/teensy-4.x/src/sam_bringup.c:
#ifdef CONFIG_VIDEO_FB
/* Initialize and register the framebuffer driver */
ret = fb_register(0, 0);
if (ret < 0)
{
syslog(LOG_ERR, "ERROR: fb_register() failed: %d\n", ret);
}
#endifThen you tight the framebuffer and LCD driver by defining function board_lcd_getdev, for example boards/arm/imxrt/teensy-4.x/src/st7789.c
struct lcd_dev_s *board_lcd_getdev(int devno)
{
g_lcd = st7789_lcdinitialize(g_spidev);
if (!g_lcd)
{
lcderr("ERROR: Failed to bind SPI port 4 to LCD %d\n", devno);
}
else
{
lcdinfo("SPI port 4 bound to LCD %d\n", devno);
return g_lcd;
}
return NULL;
}and you have the full framebuffer access, because you registered /dev/fb0 and you application can access it. The framebuffer adapter (drivers/lcd/lcd_framebuffer.c) provides the interface between framebuffer API calls and LCD API.
The whole point is the LCD controllers provide include/nuttx/lcd/lcd.h interface as putarea, getarea, putrun etc. and this interface is common whether you use framebuffer adapter or LCD character device driver. Therefore the user can decide what interface he needs/wants and we have just one common driver implementation in the source code. Yes, there is some overhead in CPU time because of more indirect calls etc.
Most of the current implementations (not just ST7789) are written as LCD device drivers, though yes, there are some exceptions with older controllers that are directly registered to dev/ without both LCD and FB API. But adding ST7796 as direct framebuffer interface will just cause someone later implementing it as LCD anyway, but unnecessary duplicating most of the code.
| bool "ST7796 LCD Display" | ||
| default n | ||
| depends on SPI | ||
| select LCD_FRAMEBUFFER |
There was a problem hiding this comment.
Enabling LCD driver shouldn't automatically enable framebuffer.
There was a problem hiding this comment.
This driver requires framebuffer infrastructure to work. Maybe I have understood wrongly. But what I have understood is that Nuttx works as an 'xor' approach for LCD and FB drivers. The signature functions are incompatibles, right?
This is correct because the driver allocates framebuffer: kmm_zalloc(xres * yres * bpp); Also we are using
fb_vtable_s interface and implementing "updatearea()" not putrun(). Finally, the driver returns at struct fb_vtable_s* rather struct lcd_dev_s*. not
/****************************************************************************
- Name: st7796_fbinitialize
- Description:
- Initialize the ST7796 LCD controller and framebuffer driver. This
- function allocates the framebuffer memory, initializes the driver
- structure, sends the display initialization sequence, and returns
- a pointer to the framebuffer virtual table for use with the NuttX
- framebuffer interface.
- Input Parameters:
- spi - Reference to the SPI driver structure to use for communication
- config - Board-specific configuration (frequency, resolution, etc.)
- Returned Value:
- On success, a pointer to the framebuffer virtual table is returned.
- On failure, NULL is returned.
****************************************************************************/
FAR struct fb_vtable_s *st7796_fbinitialize(FAR struct spi_dev_s *spi,
FAR const struct st7796_config_s
*config)
{
This driver also uses the fb.h interfaces, examples:
#include <nuttx/video/fb.h>
struct fb_videoinfo_s
struct fb_planeinfo_s
struct fb_area_s
And such thing just can be used once the symbol "CONFIG_VIDEO_FB" is enabled via menuconfig.
Finally, the ddriver implements the framebuffer driver (not the lcd one):
priv->vtable.getvideoinfo = st7796_getvideoinfo; // fb interface
priv->vtable.getplaneinfo = st7796_getplaneinfo; // fb interface
priv->vtable.updatearea = st7796_updatearea; // fb interface
priv->vtable.ioctl = st7796_ioctl; // fb interface
So, in the end the ST7789 CAN work without a FB but ST7796 CAN NOT. This is an architecture decision. Hence, we must enable this symbol to use it. Did you understand my points?
There was a problem hiding this comment.
You should use 'depends on' instead of 'select'
There was a problem hiding this comment.
Thank you! I'll change it as well :)
| SPI_CMDDATA(priv->spi, SPIDEV_DISPLAY(0), true); | ||
| SPI_SEND(priv->spi, cmd); | ||
| #else | ||
| # error "CONFIG_SPI_CMDDATA must be enabled for ST7796" |
There was a problem hiding this comment.
I would move the error at the beginning of the file and remove CONFIG_SPI_CMDDATA guards from the rest of the code to avoid the duplication
There was a problem hiding this comment.
Hello. Yes, I totally agree on this one. So when I put this 'guards' at the beggining of the code, it will fail even before to process the code. I will change it! Thank you for your feedback.
|
So, let me organize the needs: 1 - Remove the duplicated guards for SPID_CMDDATA; # error "CONFIG_SPI_CMDDATA must be enabled for ST7796" 2 - Remove the redundancy check "#ifdef CONFIG_LCD_ST7796". 3 - Fix Kconfig: 4 - removing from public api to .c: 4.1 - keep in public api 5 - Let me share also images and logs. 
|
|
|
||
| /* Command sequence entry for initialization */ | ||
|
|
||
| struct st7796_cmd_s |
There was a problem hiding this comment.
This, and all the macro definitions for registers and such things, should go in the C file so they're private.
There was a problem hiding this comment.
I agree, in parts.
To remove:
I have one question. Should I remove from .h all internal registers? Ex:
ST7796_NOP
ST7796_SWRESET
ST7796_RDDID
ST7796_RDDST
ST7796_SLPIN
ST7796_SLPOUT
ST7796_PTLON
ST7796_NORON
ST7796_RDIMGFMT
etc...
I'll also move: Individual MADCTL bit definitions (ST7796_MADCTL_MY, MX, MV, etc.) and struct st7796_cmd_s (internal command sequence structure).
To keep:
Basically the ST7796_XRES_RAW and ST7796_YRES_RAW are part of the public API because board code needs them to calculate orientation-dependent resolution at compile-time based on Kconfig settings.
So, I have created a port for STM32H753ZI (from stm32_st7796.c):
#if defined(CONFIG_NUCLEO_H753ZI_ST7796_LANDSCAPE) || \
defined(CONFIG_NUCLEO_H753ZI_ST7796_RLANDSCAPE)
# define ST7796_XRES ST7796_YRES_RAW /* 480 */
# define ST7796_YRES ST7796_XRES_RAW /* 320 */
#else
# define ST7796_XRES ST7796_XRES_RAW /* 320 */
# define ST7796_YRES ST7796_YRES_RAW /* 480 */
#endif
From my understanding this have to be at the .h because the board must access it. The board shall pass xres and yres via st7796_config_s. Indeed there is a alternative for it (wrong decision from my view): hardcode it as below:
.xres = 480,
.yres = 320,
Without these, board files would need to hardcode dimensions, breaking portability across different ST7796 variants (e.g., 320x480 vs 480x320 vs 240x320)."
The same is applicable for the other #defines. Actually I came across such situation in my 'pcb'. I realized that I mounted it upside down and actually, it was nice to see it and develop such thing to fix it quickly.
#if defined(CONFIG_NUCLEO_H753ZI_ST7796_LANDSCAPE)
# ifdef CONFIG_NUCLEO_H753ZI_ST7796_BGR
# define ST7796_MADCTL_BASE ST7796_MADCTL_LANDSCAPE_BGR
# else
# define ST7796_MADCTL_BASE ST7796_MADCTL_LANDSCAPE
# endif
#elif
In the end, with such parameters at .h, the board code must select the appropriate MADCTL value based on hardware orientation and RGB/BGR panel type configured via Kconfig. These macros abstract the hardware-specific bit manipulation from board code. The alternative would require board files to manually construct MADCTL values using bit operations, which violates encapsulation and makes the code unmaintainable when hardware details change.
Finally, the same is applicable for the spi frequency.
#ifndef CONFIG_NUCLEO_H753ZI_ST7796_FREQUENCY
# define CONFIG_NUCLEO_H753ZI_ST7796_FREQUENCY ST7796_SPI_MAXFREQUENCY
#endif
Board code uses this as a default when users don't specify a custom frequency via Kconfig. This ensures boards use safe values by default while allowing as well the users to override it when needed. Without this, every board would need to duplicate the datasheet value, risking errors and maintenance burden when hardware specs change and the same reasoning is applicable for many other macros.
conclusion?
Hence, to sumarize it, I agree to move internal implementation details (register commands, bit definitions, internal structures) to the .c file. This is the right approach. But I disagree with moving these macros to the .c file, as they are part of the public configuration API:
ST7796_XRES_RAW
ST7796_YRES_RAW
ST7796_SPI_MAXFREQUENCY
ST7796_MADCTL_PORTRAIT
ST7796_MADCTL_PORTRAIT_BGR
ST7796_MADCTL_RPORTRAIT
ST7796_MADCTL_RPORTRAIT_BGR
ST7796_MADCTL_LANDSCAPE
ST7796_MADCTL_LANDSCAPE_BGR
ST7796_MADCTL_RLANDSCAPE
ST7796_MADCTL_RLANDSCAPE_BGR
These aren't implementation details leaking into the API - they are the API for board-specific hardware configuration.
There was a problem hiding this comment.
I agree, in parts.
To remove: I have one question. Should I remove from .h all internal registers? Ex:
ST7796_NOP ST7796_SWRESET ST7796_RDDID ST7796_RDDST ST7796_SLPIN ST7796_SLPOUT ST7796_PTLON ST7796_NORON ST7796_RDIMGFMT etc...
Yes, anything that isn't needed by the user including your header file should not be exposed. Registers are internally used only; your LCD driver is interacted with by users through the public functions only.
I'll also move: Individual MADCTL bit definitions (ST7796_MADCTL_MY, MX, MV, etc.) and struct st7796_cmd_s (internal command sequence structure).
To keep: Basically the ST7796_XRES_RAW and ST7796_YRES_RAW are part of the public API because board code needs them to calculate orientation-dependent resolution at compile-time based on Kconfig settings.
That seems reasonable if there's no way around it!
So, I have created a port for STM32H753ZI (from stm32_st7796.c):
#if defined(CONFIG_NUCLEO_H753ZI_ST7796_LANDSCAPE) || \ defined(CONFIG_NUCLEO_H753ZI_ST7796_RLANDSCAPE) # define ST7796_XRES ST7796_YRES_RAW /* 480 */ # define ST7796_YRES ST7796_XRES_RAW /* 320 */ #else # define ST7796_XRES ST7796_XRES_RAW /* 320 */ # define ST7796_YRES ST7796_YRES_RAW /* 480 */ #endifFrom my understanding this have to be at the .h because the board must access it. The board shall pass xres and yres via st7796_config_s. Indeed there is a alternative for it (wrong decision from my view): hardcode it as below:
.xres = 480, .yres = 320,
Without these, board files would need to hardcode dimensions, breaking portability across different ST7796 variants (e.g., 320x480 vs 480x320 vs 240x320)."
That seems totally fine, go with your macro-define method.
The same is applicable for the other #defines. Actually I came across such situation in my 'pcb'. I realized that I mounted it upside down and actually, it was nice to see it and develop such thing to fix it quickly.
#if defined(CONFIG_NUCLEO_H753ZI_ST7796_LANDSCAPE) # ifdef CONFIG_NUCLEO_H753ZI_ST7796_BGR # define ST7796_MADCTL_BASE ST7796_MADCTL_LANDSCAPE_BGR # else # define ST7796_MADCTL_BASE ST7796_MADCTL_LANDSCAPE # endif #elifIn the end, with such parameters at .h, the board code must select the appropriate MADCTL value based on hardware orientation and RGB/BGR panel type configured via Kconfig. These macros abstract the hardware-specific bit manipulation from board code. The alternative would require board files to manually construct MADCTL values using bit operations, which violates encapsulation and makes the code unmaintainable when hardware details change.
Finally, the same is applicable for the spi frequency.
#ifndef CONFIG_NUCLEO_H753ZI_ST7796_FREQUENCY # define CONFIG_NUCLEO_H753ZI_ST7796_FREQUENCY ST7796_SPI_MAXFREQUENCY #endifBoard code uses this as a default when users don't specify a custom frequency via Kconfig. This ensures boards use safe values by default while allowing as well the users to override it when needed. Without this, every board would need to duplicate the datasheet value, risking errors and maintenance burden when hardware specs change and the same reasoning is applicable for many other macros.
You can set the default value in Kconfig instead by using default 1000
(replace 1000 with your frequency). This is the preferred method.
conclusion? Hence, to sumarize it, I agree to move internal implementation details (register commands, bit definitions, internal structures) to the .c file. This is the right approach. But I disagree with moving these macros to the .c file, as they are part of the public configuration API:
ST7796_XRES_RAW ST7796_YRES_RAW ST7796_SPI_MAXFREQUENCY ST7796_MADCTL_PORTRAIT ST7796_MADCTL_PORTRAIT_BGR ST7796_MADCTL_RPORTRAIT ST7796_MADCTL_RPORTRAIT_BGR ST7796_MADCTL_LANDSCAPE ST7796_MADCTL_LANDSCAPE_BGR ST7796_MADCTL_RLANDSCAPE ST7796_MADCTL_RLANDSCAPE_BGR
That's fine, I only mean that you should move anything not needed by the user to the private C file.
6 participants
Add support for ST7796 TFT LCD controller (320x480). The driver implements the NuttX framebuffer interface for SPI-connected displays.
Features:
The driver uses a board-provided configuration structure allowing flexible setup of resolution, SPI frequency, color depth, and initial MADCTL value without requiring Kconfig options in the generic driver.
Tested with LVGL graphics library on STM32H7 nucleo0h753zi platform.
Note: Please adhere to Contributing Guidelines.
Summary
Add ST7796 TFT LCD framebuffer driver for 320x480 SPI-connected displays.
The ST7796 is a popular TFT controller used in 3.5" IPS LCD modules. This driver implements the NuttX framebuffer interface (fb_vtable_s) allowing integration with graphics libraries like LVGL.
Key features:
SPI interface using CONFIG_SPI_CMDDATA for D/C pin control
RGB565 (16-bit) and RGB666 (18-bit) color formats
Runtime rotation support (0, 90, 180, 270 degrees) via MADCTL register
Board-provided configuration via st7796_config_s structure
Partial screen update via updatearea for efficient rendering
Persistent swap buffer to avoid per-frame memory allocations
The driver follows the same pattern as existing LCD drivers (ST7789, GC9A01) and does not introduce board-specific dependencies in the generic driver code.
Impact
New driver: drivers/lcd/st7796.c
New header: include/nuttx/lcd/st7796.h
Updated: drivers/lcd/Kconfig, Make.defs, CMakeLists.txt
No breaking changes to existing code
Requires CONFIG_SPI_CMDDATA enabled for D/C pin control
Testing
Hardware: STM32H753ZI Nucleo + 3.5" ST7796 IPS LCD (SPI)
NuttX version: 12.12
Tested with:
fb example (framebuffer color test)
LVGL graphics library (animations, widgets)
Runtime rotation via FBIOSET_ROTATION ioctl
Build configurations: nsh, fb, lvgl
PRs without testing information will not be accepted. We will
request test logs.