Re: [PATCH 2/2] drm/panel: Add driver for sitronix ST7789V panel

[Thierry Reding]

On Mon, Feb 06, 2017 at 01:13:19PM +0100, Noralf TrÃnnes wrote:
> 
> Den 06.02.2017 11.50, skrev Maxime Ripard:
> > Hi Noralf,
> > 
> > On Fri, Feb 03, 2017 at 07:53:58PM +0100, Noralf TrÃnnes wrote:
> > > Den 03.02.2017 10.59, skrev Maxime Ripard:
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
> > > > ---
> > > >    drivers/gpu/drm/panel/Kconfig                  |   4 +-
> > > >    drivers/gpu/drm/panel/Makefile                 |   1 +-
> > > >    drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 435 ++++++++++++++++++-
> > > >    3 files changed, 440 insertions(+), 0 deletions(-)
> > > >    create mode 100644 drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > 
> > > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > > > index 62aba976e744..d07b996a07b8 100644
> > > > --- a/drivers/gpu/drm/panel/Kconfig
> > > > +++ b/drivers/gpu/drm/panel/Kconfig
> > > > @@ -81,4 +81,8 @@ config DRM_PANEL_SHARP_LS043T1LE01
> > > >    	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
> > > >    	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
> > > > +config DRM_PANEL_SITRONIX_ST7789V
> > > > +	tristate "Sitronx ST7789V panel"
> > > > +	depends on OF && SPI
> > > > +
> > > >    endmenu
> > > > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > > > index a5c7ec0236e0..41b245d39984 100644
> > > > --- a/drivers/gpu/drm/panel/Makefile
> > > > +++ b/drivers/gpu/drm/panel/Makefile
> > > > @@ -6,3 +6,4 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
> > > >    obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
> > > >    obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> > > >    obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> > > > +obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > new file mode 100644
> > > > index 000000000000..aab817b55aa6
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > @@ -0,0 +1,435 @@
> > > > +/*
> > > > + * Copyright (C) 2017 Free Electrons
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License version
> > > > + * 2 as published by the Free Software Foundation.
> > > > + */
> > > > +
> > > > +#include <drm/drmP.h>
> > > > +#include <drm/drm_panel.h>
> > > > +
> > > > +#include <linux/gpio/consumer.h>
> > > > +#include <linux/spi/spi.h>
> > > > +
> > > > +#define ST7789V_SLPIN_CMD		0x10
> > > > +
> > > > +#define ST7789V_SLPOUT_CMD		0x11
> > > > +
> > > > +#define ST7789V_INVON_CMD		0x21
> > > > +
> > > > +#define ST7789V_DISPOFF_CMD		0x28
> > > > +
> > > > +#define ST7789V_DISPON_CMD		0x29
> > > > +
> > > > +#define ST7789V_MADCTL_CMD		0x36
> > > > +
> > > > +#define ST7789V_COLMOD_CMD		0x3a
> > > You can use these from include/video/mipi_display.h:
> > > 
> > >          MIPI_DCS_ENTER_SLEEP_MODE       = 0x10,
> > >          MIPI_DCS_EXIT_SLEEP_MODE        = 0x11,
> > >          MIPI_DCS_ENTER_INVERT_MODE      = 0x21,
> > >          MIPI_DCS_SET_DISPLAY_OFF        = 0x28,
> > >          MIPI_DCS_SET_DISPLAY_ON         = 0x29,
> > >          MIPI_DCS_SET_ADDRESS_MODE       = 0x36,
> > >          MIPI_DCS_SET_PIXEL_FORMAT       = 0x3A,
> > I don't know, that controller cannot do MIPI DSI at all, and we would
> > have to define all the other commands anyway.
> 
> MIPI DCS commands can be sent over both Display Serial Interface (DSI)
> and in your case Display Bus Interface (DBI) (not quite sure if the
> Control Interface can be outside of DBI). So Display Command Set (DCS)
> is not specific to the interface, but to the controller.
> 
> It's up to you if you want to use them though.

I think it makes sense to unify as much as possible to make it easier to
spot similarities between controllers. Also, we've already got these
definitions in one place, no use in duplicating them. They are obviously
used for the same purposes.

Thierry

Attachment: signature.asc
Description: PGP signature