Hi Sebastian, Thanks for your work and for the beautiful timing :-) On 3/18/23 00:23, Sebastian Reichel wrote: > UNI-T UTi260b has a Inanbo T28CP45TN89 v17 panel. I could not find > proper documentation for the panel apart from a technical drawing, but > according to the vendor U-Boot it is based on a Sitronix st7789v chip. > I generated the init sequence by modifying the default one until proper > graphics output has been seen on the device. I can spot only a few differences: - bits per pixel: 18 (RGB666) vs. 16 (RGB565) - invert mode - sync/clk signal polarity The init sequences are largely the same, which leads to vast code duplication. Instead, the st7789v_prepare could be adjusted to consider the st7789v_panel_info and apply the required settings accordingly. For example, the polarities could be embedded into the drm_display_mode structure... > Signed-off-by: Sebastian Reichel <sre@xxxxxxxxxx> > --- > .../gpu/drm/panel/panel-sitronix-st7789v.c | 137 ++++++++++++++++++ > 1 file changed, 137 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > index a62a2f5737e4..90f70eb84f11 100644 > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > @@ -10,6 +10,7 @@ > #include <linux/spi/spi.h> > > #include <video/mipi_display.h> > +#include <linux/media-bus-format.h> > > #include <drm/drm_device.h> > #include <drm/drm_modes.h> > @@ -113,6 +114,8 @@ struct st7789v; > struct st7789_panel_info { > const struct drm_display_mode *mode; > int (*init_sequence)(struct st7789v *ctx); > + unsigned int bpc; > + u32 bus_format; ... and here you introduce fields for the bits per pixel. Just a field for the invert mode is missing. BTW, I would introduce these fields in the previous patch. This patch should be only about filling out the already existing fields for the new panel. > }; > > struct st7789v { > @@ -174,6 +177,20 @@ static const struct drm_display_mode default_mode = { > .height_mm = 103, > }; > > +static const struct drm_display_mode t28cp45tn89_mode = { > + .clock = 6008, > + .hdisplay = 240, > + .hsync_start = 240 + 38, > + .hsync_end = 240 + 38 + 10, > + .htotal = 240 + 38 + 10 + 10, > + .vdisplay = 320, > + .vsync_start = 320 + 8, > + .vsync_end = 320 + 8 + 4, > + .vtotal = 320 + 8 + 4 + 4, > + .width_mm = 43, > + .height_mm = 57, > +}; > + > static int init_sequence_default(struct st7789v *ctx) { > int ret; > > @@ -283,11 +300,125 @@ static int init_sequence_default(struct st7789v *ctx) { > return 0; > } > > +static int init_sequence_t28cp45tn89(struct st7789v *ctx) { > + int ret; > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, > + MIPI_DCS_SET_ADDRESS_MODE)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, > + MIPI_DCS_SET_PIXEL_FORMAT)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, > + (MIPI_DCS_PIXEL_FMT_16BIT << 4) | > + (MIPI_DCS_PIXEL_FMT_16BIT))); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_PORCTRL_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0xc)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0xc)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PORCTRL_IDLE_BP(3) | > + ST7789V_PORCTRL_IDLE_FP(3))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, > + ST7789V_PORCTRL_PARTIAL_BP(3) | > + ST7789V_PORCTRL_PARTIAL_FP(3))); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_GCTRL_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_GCTRL_VGLS(5) | > + ST7789V_GCTRL_VGHS(3))); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_VCOMS_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0x2b)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_LCMCTRL_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_LCMCTRL_XMH | > + ST7789V_LCMCTRL_XMX | > + ST7789V_LCMCTRL_XBGR)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_VDVVRHEN_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_VDVVRHEN_CMDEN)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_VRHS_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0xf)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_VDVS_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0x20)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_FRCTRL2_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, 0xf)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_PWCTRL1_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PWCTRL1_MAGIC)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PWCTRL1_AVDD(2) | > + ST7789V_PWCTRL1_AVCL(2) | > + ST7789V_PWCTRL1_VDS(1))); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_PVGAMCTRL_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP63(0xd))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP1(0xca))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP2(0xe))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP4(8))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP6(9))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP13(7))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP20(0x2d))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP27(0xb) | > + ST7789V_PVGAMCTRL_VP36(3))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP43(0x3d))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_JP1(3) | > + ST7789V_PVGAMCTRL_VP50(4))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP57(0xa))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP59(0xa))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP61(0x1b))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_PVGAMCTRL_VP62(0x28))); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_NVGAMCTRL_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN63(0xd))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN1(0xca))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN2(0xf))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN4(8))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN6(8))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN13(7))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN20(0x2e))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN27(0xc) | > + ST7789V_NVGAMCTRL_VN36(5))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN43(0x40))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_JN1(3) | > + ST7789V_NVGAMCTRL_VN50(4))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN57(9))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN59(0xb))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN61(0x1b))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_NVGAMCTRL_VN62(0x28))); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, MIPI_DCS_EXIT_INVERT_MODE)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_RAMCTRL_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RAMCTRL_DM_RGB | > + ST7789V_RAMCTRL_RM_RGB)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RAMCTRL_EPF(3) | > + ST7789V_RAMCTRL_MAGIC)); > + > + ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_RGBCTRL_CMD)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_WO | > + ST7789V_RGBCTRL_RCM(2) | > + ST7789V_RGBCTRL_VSYNC_HIGH)); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_VBP(8))); > + ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_HBP(20))); > + > + return 0; > +} > + > struct st7789_panel_info default_panel = { > .mode = &default_mode, > .init_sequence = init_sequence_default, > }; > > +struct st7789_panel_info t28cp45tn89_panel = { > + .mode = &t28cp45tn89_mode, > + .init_sequence = init_sequence_t28cp45tn89, > + .bpc = 6, > + .bus_format = MEDIA_BUS_FMT_RGB565_1X16, > +}; > + > static int st7789v_get_modes(struct drm_panel *panel, > struct drm_connector *connector) > { > @@ -307,8 +438,12 @@ static int st7789v_get_modes(struct drm_panel *panel, > mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > drm_mode_probed_add(connector, mode); > > + connector->display_info.bpc = ctx->info->bpc; > connector->display_info.width_mm = ctx->info->mode->width_mm; > connector->display_info.height_mm = ctx->info->mode->height_mm; > + if (ctx->info->bus_format) > + drm_display_info_set_bus_formats(&connector->display_info, > + &ctx->info->bus_format, 1); This should also be in a previous patch. Best regards, Michael > > return 1; > } > @@ -417,12 +552,14 @@ static void st7789v_remove(struct spi_device *spi) > > static const struct spi_device_id st7789v_spi_id[] = { > { "st7789v", (unsigned long) &default_panel }, > + { "t28cp45tn89-v17", (unsigned long) &t28cp45tn89_panel }, > { } > }; > MODULE_DEVICE_TABLE(spi, st7789v_spi_id); > > static const struct of_device_id st7789v_of_match[] = { > { .compatible = "sitronix,st7789v", .data = &default_panel }, > + { .compatible = "inanbo,t28cp45tn89-v17", .data = &t28cp45tn89_panel }, > { } > }; > MODULE_DEVICE_TABLE(of, st7789v_of_match);