Re: [RFC PATCH] drm/sitronix: add ST7789V panel driver

[Archit Anant]

Hi ChenYu,

Thank you for the detailed review and the pointers toward the documentation.
I will ensure the headers are sorted alphabetically and the driver name
conflict is resolved in the next iteration.

> The reset logic in mipi_dbi is inverted when compared to panel-st7789v.
> mipi_dbi needs to be taught the "proper" reset polarity.

Noted. I will look into the mipi_dbi core to see how to handle the reset
polarity correctly.

> Instead this functionality could be merged into the existing panel-st7789v
> driver. You mentioned above that that driver only supports the 9-bit SPI
> transfer mode. However porting that driver over to mipi_dbi would fix this,
> and remove some redundant code. And tinydrm support could be added on top
> of that.
>
> I actually mentioned I was going to work on this on IRC. But I only ported
> the driver over to use mipi_dbi, and haven't gotten around to adding
> tinydrm support. I can send out the conversion patches if that helps
> you.

That would be fantastic and would save a lot of redundant effort. If you
send out the patches to convert the existing panel-st7789v driver to
mipi_dbi, I would be happy to build the 'tiny' (simple display pipe)
support on top of your series.

Please CC me when you post them, and I will rebase my work accordingly.

Best regards,
Archit

On Sat, Feb 14, 2026 at 6:35 PM Chen-Yu Tsai <wens@xxxxxxxxxx> wrote:
>
> On Sat, Feb 14, 2026 at 5:21 PM Archit Anant <architanant5@xxxxxxxxx> wrote:
> >
> > Add a DRM driver for Sitronix ST7789V display controllers using the
> > mipi_dbi interface.
> >
> > Currently, support for this controller is split between a legacy fbdev
> > driver in staging (fb_st7789v.c) and a DRM panel driver that requires
> > 9-bit SPI words (panel-sitronix-st7789v.c). This new driver uses the
> > mipi_dbi helper to support standard 8-bit SPI with a D/C GPIO, which
> > is the configuration used by the vast majority of hobbyist and
> > embedded hardware.
>
> Notes about this below.
>
> >
> > The initialization sequence is ported from the staging driver and
> > supports several panels:
> > - Generic 240x320 profile
> > - HannStar HSD20 IPS
> > - Inanbo T28CP45TN89-V17
> > - EDT ET028013DMA
> > - Jasonic JT240MHQS-HWT-EK-E3
>
> First of all, please run scripts/checkpatch.pl on patches before you send
> them. The indentation is all wrong. The script would have caught it.
>
> And check out Documentation/process/submitting-patches.rst for any other
> steps you should do before submitting patches.
>
> > Signed-off-by: Archit Anant <architanant5@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/sitronix/Kconfig   |  17 ++
> >  drivers/gpu/drm/sitronix/Makefile  |   2 +
> >  drivers/gpu/drm/sitronix/st7789v.c | 307 +++++++++++++++++++++++++++++
> >  3 files changed, 326 insertions(+)
> >  create mode 100644 drivers/gpu/drm/sitronix/st7789v.c
> >
> > diff --git a/drivers/gpu/drm/sitronix/Kconfig b/drivers/gpu/drm/sitronix/Kconfig
> > index 6de7d92d9b74..7a2c66677003 100644
> > --- a/drivers/gpu/drm/sitronix/Kconfig
> > +++ b/drivers/gpu/drm/sitronix/Kconfig
> > @@ -40,3 +40,20 @@ config DRM_ST7735R
> >
> >           If M is selected the module will be called st7735r.
> >
> > +config DRM_ST7789V
> > +       tristate "DRM support for Sitronix ST7789V display panels"
> > +       depends on DRM && SPI
> > +       select DRM_CLIENT_SELECTION
> > +       select DRM_GEM_DMA_HELPER
> > +       select DRM_KMS_HELPER
> > +       select DRM_MIPI_DBI
> > +       select BACKLIGHT_CLASS_DEVICE
> > +       help
> > +         DRM driver for Sitronix ST7789V panels connected via SPI.
> > +         This driver supports several panels including:
> > +         * HannStar HSD20 IPS
> > +         * Inanbo T28CP45TN89-V17
> > +         * EDT ET028013DMA
> > +         * Jasonic JT240MHQS-HWT-EK-E3
> > +
> > +         If M is selected the module will be called st7789v.
> > diff --git a/drivers/gpu/drm/sitronix/Makefile b/drivers/gpu/drm/sitronix/Makefile
> > index bd139e5a6995..96b2a4d85368 100644
> > --- a/drivers/gpu/drm/sitronix/Makefile
> > +++ b/drivers/gpu/drm/sitronix/Makefile
> > @@ -1,3 +1,5 @@
> >  obj-$(CONFIG_DRM_ST7571_I2C)           += st7571-i2c.o
> >  obj-$(CONFIG_DRM_ST7586)               += st7586.o
> >  obj-$(CONFIG_DRM_ST7735R)              += st7735r.o
> > +obj-$(CONFIG_DRM_ST7789V)              += st7789v.o
> > +
>
> Empty line not needed.
>
> > diff --git a/drivers/gpu/drm/sitronix/st7789v.c b/drivers/gpu/drm/sitronix/st7789v.c
> > new file mode 100644
> > index 000000000000..4ce4b46d8df2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sitronix/st7789v.c
> > @@ -0,0 +1,307 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * DRM driver for Sitronix ST7789V LCD panels
> > + *
> > + * Copyright (C) 2026 Archit Anant <architanant5@xxxxxxxxx>
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/delay.h>
> > +#include <linux/backlight.h>
>
> Sort this group alphabetically.
>
> > +
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_managed.h>
> > +#include <drm/drm_mipi_dbi.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_gem_dma_helper.h>
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/clients/drm_client_setup.h>
> > +#include <drm/drm_fbdev_dma.h>
>
> Sort this group alphabetically.
>
> And add an empty line here for group separation.
>
> > +#include <video/mipi_display.h>
> > +
> > +#define ST7789V_PORCTRL      0xb2
> > +#define ST7789V_GCTRL        0xb7
> > +#define ST7789V_VCOMS        0xbb
> > +#define ST7789V_LCMCTRL      0xc0
> > +#define ST7789V_VDVVRHEN     0xc2
> > +#define ST7789V_VRHS         0xc3
> > +#define ST7789V_VDVS         0xc4
> > +#define ST7789V_VCMOFSET     0xc5
> > +#define ST7789V_FRCTRL2      0xc6
> > +#define ST7789V_PWCTRL1      0xd0
> > +#define ST7789V_PVGAMCTRL    0xe0
> > +#define ST7789V_NVGAMCTRL    0xe1
> > +
> > +#define ST7789V_MADCTL_MY  BIT(7)
> > +#define ST7789V_MADCTL_MX  BIT(6)
> > +#define ST7789V_MADCTL_MV  BIT(5)
> > +#define ST7789V_MADCTL_BGR BIT(3)
> > +
> > +
> > +struct st7789v_cfg {
> > +       const struct drm_display_mode mode;
> > +       unsigned int left_offset;
> > +       unsigned int top_offset;
> > +       bool is_ips;   /* Controls PORCTRL and GCTRL timings */
> > +       bool invert;   /* Controls Color Inversion (positive/negative) */
> > +};
> > +struct st7789v_priv {
> > +       struct mipi_dbi_dev dbidev; /* Must be first for .release() */
> > +       const struct st7789v_cfg *cfg;
> > +};
> > +
> > +
> > +/* 1. Generic Fallback (Matches default behavior of fb_st7789v.c) */
> > +static const struct st7789v_cfg generic_cfg = {
> > +       .mode = { DRM_SIMPLE_MODE(240, 320, 0, 0) },
> > +       .is_ips = false,
> > +       .invert = true,
> > +};
> > +
> > +/* 2. HannStar 2.0" IPS (The specific panel handled in staging) */
> > +static const struct st7789v_cfg hsd20_ips_cfg = {
> > +       .mode = { DRM_SIMPLE_MODE(240, 320, 31, 41) },
> > +       .is_ips = true,
> > +       .invert = true,
> > +};
> > +
> > +/* 3. Inanbo 2.8" (From the 9-bit driver: No Inversion) */
> > +static const struct st7789v_cfg inanbo_panel_cfg = {
> > +       .mode = { DRM_SIMPLE_MODE(240, 320, 43, 57) },
> > +       .is_ips = false,
> > +       .invert = false,
> > +};
> > +
> > +/* 4. EDT 2.8" (From the 9-bit driver: Normal Inversion) */
> > +static const struct st7789v_cfg edt_panel_cfg = {
> > +       .mode = { DRM_SIMPLE_MODE(240, 320, 43, 58) },
> > +       .is_ips = false,
> > +       .invert = true,
> > +};
> > +
> > +/* 5. Jasonic 2.4" (From the 9-bit driver: Custom Height + Offset) */
> > +static const struct st7789v_cfg jasonic_panel_cfg = {
> > +       .mode = { DRM_SIMPLE_MODE(240, 280, 37, 43) },
> > +       .is_ips = true,
> > +       .invert = true,
> > +       .top_offset = 38,
> > +};
> > +
> > +DEFINE_DRM_GEM_DMA_FOPS(st7789v_fops);
> > +
> > +static const struct drm_driver st7789v_driver = {
> > +    .driver_features    = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> > +    .fops               = &st7789v_fops,
> > +    DRM_GEM_DMA_DRIVER_OPS_VMAP,
> > +    DRM_FBDEV_DMA_DRIVER_OPS,
> > +    .debugfs_init       = mipi_dbi_debugfs_init,
> > +    .name               = "st7789v",
> > +    .desc               = "Sitronix ST7789V",
> > +    .major              = 1,
> > +    .minor              = 0,
> > +};
> > +
> > +static void st7789v_pipe_enable(struct drm_simple_display_pipe *pipe,
> > +                                struct drm_crtc_state *crtc_state,
> > +                                struct drm_plane_state *plane_state)
> > +{
> > +    struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
> > +    struct st7789v_priv *priv = container_of(dbidev, struct st7789v_priv, dbidev);
> > +    struct mipi_dbi *dbi = &dbidev->dbi;
> > +    int ret,idx;
> > +
> > +    if (!drm_dev_enter(pipe->crtc.dev, &idx))
> > +        return;
> > +
> > +    ret = mipi_dbi_poweron_reset(dbidev);
> > +    if (ret)
> > +               goto out_exit;
> > +
> > +    mipi_dbi_command(dbi, MIPI_DCS_SOFT_RESET);
> > +    msleep(150);
> > +
> > +    mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
> > +    msleep(500);
> > +
> > +    mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
> > +
> > +    if (priv->cfg->is_ips) {
> > +        mipi_dbi_command(dbi, ST7789V_PORCTRL, 0x05, 0x05, 0x00, 0x33, 0x33);
> > +        mipi_dbi_command(dbi, ST7789V_GCTRL, 0x75);
> > +    } else {
> > +        mipi_dbi_command(dbi, ST7789V_PORCTRL, 0x0c, 0x0c, 0x00, 0x33, 0x33);
> > +        mipi_dbi_command(dbi, ST7789V_GCTRL, 0x35);
> > +    }
> > +
> > +    mipi_dbi_command(dbi, ST7789V_VCOMS, 0x20);
> > +    mipi_dbi_command(dbi, ST7789V_LCMCTRL, 0x2c);
> > +    mipi_dbi_command(dbi, ST7789V_VDVVRHEN, 0x01);
> > +    mipi_dbi_command(dbi, ST7789V_VRHS, 0x12);
> > +    mipi_dbi_command(dbi, ST7789V_VDVS, 0x20);
> > +    mipi_dbi_command(dbi, ST7789V_FRCTRL2, 0x0f);
> > +    mipi_dbi_command(dbi, ST7789V_PWCTRL1, 0xa4, 0xa1);
> > +
> > +    mipi_dbi_command(dbi, ST7789V_PVGAMCTRL,
> > +                     0xd0, 0x04, 0x0d, 0x11, 0x13, 0x2b, 0x3f, 0x54,
> > +                     0x4c, 0x18, 0x0d, 0x0b, 0x1f, 0x23);
> > +    mipi_dbi_command(dbi, ST7789V_NVGAMCTRL,
> > +                     0xd0, 0x04, 0x0c, 0x11, 0x13, 0x2c, 0x3f, 0x44,
> > +                     0x51, 0x2f, 0x1f, 0x1f, 0x20, 0x23);
> > +
> > +    if (priv->cfg->invert)
> > +        mipi_dbi_command(dbi, MIPI_DCS_ENTER_INVERT_MODE);
> > +    else
> > +        mipi_dbi_command(dbi, MIPI_DCS_EXIT_INVERT_MODE);
> > +    mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
> > +    msleep(100);
> > +
> > +    u8 addr_mode = 0;
> > +
> > +    switch (dbidev->rotation) {
> > +    case 90:
> > +
> > +        addr_mode = ST7789V_MADCTL_MV | ST7789V_MADCTL_MY;
> > +        break;
> > +    case 180:
> > +        addr_mode = ST7789V_MADCTL_MX | ST7789V_MADCTL_MY;
> > +        break;
> > +    case 270:
> > +        addr_mode = ST7789V_MADCTL_MV | ST7789V_MADCTL_MX;
> > +        break;
> > +    default:
> > +        addr_mode = 0;
> > +        break;
> > +    }
> > +
> > +    addr_mode |= ST7789V_MADCTL_BGR;
> > +
> > +    mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
> > +
> > +    mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
> > +
> > +out_exit:
> > +    drm_dev_exit(idx);
> > +}
> > +
> > +static const struct drm_simple_display_pipe_funcs st7789v_pipe_funcs =
> > +{
> > +    DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(st7789v_pipe_enable),
> > +};
> > +
> > +static int st7789v_probe(struct spi_device *spi)
> > +{
> > +    struct device *dev = &spi->dev;
> > +    const struct st7789v_cfg *cfg;
> > +    struct mipi_dbi_dev *dbidev;
> > +    struct st7789v_priv *priv;
> > +    struct drm_device *drm;
> > +    struct mipi_dbi *dbi;
> > +    struct gpio_desc *dc;
> > +    u32 rotation = 0;
> > +    int ret;
> > +
> > +    cfg = device_get_match_data(&spi->dev);
> > +
> > +       if (!cfg)
> > +               cfg = (void *)spi_get_device_id(spi)->driver_data;
> > +
> > +       priv = devm_drm_dev_alloc(dev, &st7789v_driver,
> > +                                 struct st7789v_priv, dbidev.drm);
> > +
> > +       if (IS_ERR(priv))
> > +               return PTR_ERR(priv);
> > +
> > +    dbidev = &priv->dbidev;
> > +    priv->cfg = cfg;
> > +
> > +    dbi = &dbidev->dbi;
> > +    drm = &dbidev->drm;
> > +
> > +    spi_set_drvdata(spi, drm);
>
> Maybe also add power supply regulator support to make it complete?
>
> > +    dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > +    if (IS_ERR(dbi->reset))
> > +        return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n");
>
> The reset logic in mipi_dbi is inverted when compared to panel-st7789v.
> mipi_dbi needs to be taught the "proper" reset polarity.
>
> > +
> > +    dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
> > +    if (IS_ERR(dc))
> > +        return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n");
>
> This should be optional, since mipi_dbi can handle the "no dc" case.
>
> > +
> > +    dbidev->backlight = devm_of_find_backlight(dev);
> > +    if (IS_ERR(dbidev->backlight))
> > +        return PTR_ERR(dbidev->backlight);
> > +
> > +    dbidev->left_offset = priv->cfg->left_offset;
> > +    dbidev->top_offset = priv->cfg->top_offset;
> > +
> > +    device_property_read_u32(dev, "rotation", &rotation);
> > +
> > +    ret = mipi_dbi_spi_init(spi, dbi, dc);
> > +    if (ret)
> > +        return ret;
> > +
> > +    ret = mipi_dbi_dev_init(dbidev, &st7789v_pipe_funcs, &cfg->mode, rotation);
> > +    if (ret)
> > +        return ret;
> > +
> > +    drm_mode_config_reset(drm);
> > +
> > +    ret = drm_dev_register(drm, 0);
> > +    if (ret)
> > +        return ret;
> > +
> > +    drm_client_setup(drm, NULL);
> > +
> > +    return 0;
> > +}
> > +
> > +static void st7789v_remove(struct spi_device *spi)
> > +{
> > +    struct drm_device *drm = spi_get_drvdata(spi);
> > +    drm_dev_unplug(drm);
> > +    drm_atomic_helper_shutdown(drm);
> > +}
> > +
> > +static void st7789v_shutdown(struct spi_device *spi)
> > +{
> > +    drm_atomic_helper_shutdown(spi_get_drvdata(spi));
> > +}
> > +
> > +
> > +static const struct of_device_id st7789v_of_match[] = {
> > +       { .compatible = "sitronix,st7789v", .data = &generic_cfg },
> > +       { .compatible = "hannstar,hsd20-ips", .data = &hsd20_ips_cfg },
> > +       { .compatible = "inanbo,t28cp45tn89-v17", .data = &inanbo_panel_cfg },
> > +       { .compatible = "edt,et028013dma", .data = &edt_panel_cfg },
> > +       { .compatible = "jasonic,jt240mhqs-hwt-ek-e3", .data = &jasonic_panel_cfg },
> > +       {  }
> > +};
> > +MODULE_DEVICE_TABLE(of, st7789v_of_match);
>
> We typically don't want two drivers matching on the same devices. The
> driver match and probe order becomes hard to predict.
>
> Instead this functionality could be merged into the existing panel-st7789v
> driver. You mentioned above that that driver only supports the 9-bit SPI
> transfer mode. However porting that driver over to mipi_dbi would fix this,
> and remove some redundant code. And tinydrm support could be added on top
> of that.
>
> I actually mentioned I was going to work on this on IRC. But I only ported
> the driver over to use mipi_dbi, and haven't gotten around to adding
> tinydrm support. I can send out the conversion patches if that helps
> you.
>
> > +
> > +static const struct spi_device_id st7789v_id[] = {
> > +    { "st7789v", 0 },
>
> You should add all the other ones as well. OF-based SPI driver module
> loading doesn't seem to work. I think that's a known issue?
>
> > +    { },
> > +};
> > +MODULE_DEVICE_TABLE(spi, st7789v_id);
> > +
> > +static struct spi_driver st7789v_spi_driver = {
> > +    .driver = {
> > +        .name = "st7789v",
>
> And you definitely can't have two drivers with the same name.
>
> > +        .of_match_table = st7789v_of_match,
> > +    },
> > +    .probe = st7789v_probe,
> > +    .remove = st7789v_remove,
> > +    .shutdown = st7789v_shutdown,
> > +    .id_table = st7789v_id,
> > +};
> > +
>
> Empty line not needed here.
>
> > +module_spi_driver(st7789v_spi_driver);
> > +
> > +MODULE_DESCRIPTION("Sitronix ST7789V DRM driver");
> > +MODULE_AUTHOR("Archit Anant <architanant5@xxxxxxxxx>");
> > +MODULE_LICENSE("GPL");
> > \ No newline at end of file
>
>     ^
>
>
> ChenYu



-- 
Sincerely,
Archit Anant