Re: [PATCH v2 5/9] drm/panel: Add driver for the NEC NL8048HL11 panel

Hi Sam,

On Sun, Aug 11, 2019 at 07:10:05PM +0200, Sam Ravnborg wrote:

On Sun, Aug 11, 2019 at 02:10:44AM +0300, Laurent Pinchart wrote:

...

This panel is used on the Zoom2/3/3630 SDP boards.

The code is based on the omapdrm-specific panel-nec-nl8048hl11 driver

Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

Changes since v1:

• Mention boards using the panel in Kconfig
• Renamed nl8048_device to nl8048_panel
• Comments updates
• Store width_mm and height_mm in drm_display_mode
• Use drm_panel_disable() in .remove() handler

---

drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 247 +++++++++++++++++++ 3 files changed, 256 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-nec-nl8048hl11.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 87cdc330672b..d28133c6aa0a 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -119,6 +119,14 @@ config DRM_PANEL_LG_LG4573 Say Y here if you want to enable support for LG4573 RGB panel. To compile this driver as a module, choose M here.

+config DRM_PANEL_NEC_NL8048HL11

• tristate "NEC NL8048HL11 RGB panel"
• depends on GPIOLIB && OF && SPI
• help

•  Say Y here if you want to enable support for the NEC NL8048HL11 RGB
  

•  panel (found on the Zoom2/3/3630 SDP boards). To compile this driver
  

•  as a module, choose M here.
  

config DRM_PANEL_NOVATEK_NT39016 tristate "Novatek NT39016 RGB/SPI panel" depends on OF && SPI diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 9bc6f3c5bf96..8052f9a7ad60 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o +obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o diff --git a/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c new file mode 100644 index 000000000000..7c1b77a0b024 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c @@ -0,0 +1,247 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • NEC NL8048HL11 Panel Driver
• • Copyright (C) 2019 Texas Instruments Incorporated
• • Based on the omapdrm-specific panel-nec-nl8048hl11 driver
• • Copyright (C) 2010 Texas Instruments Incorporated
• • Author: Erik Gilling konkers@android.com
• */

+#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/pm.h> +#include <linux/spi/spi.h>

+#include <drm/drm_connector.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h>

+struct nl8048_panel {

• struct drm_panel panel;
• struct spi_device *spi;
• struct gpio_desc *reset_gpio;

+};

+#define to_nl8048_device(p) container_of(p, struct nl8048_panel, panel)

+static int nl8048_write(struct nl8048_panel *lcd, unsigned char addr,

• 	unsigned char value)
  

+{

• u8 data[4] = { value, 0x01, addr, 0x00 };
• int ret;
• ret = spi_write(lcd->spi, data, sizeof(data));
• if (ret)

• dev_err(&lcd->spi->dev, "SPI write to %u failed: %d\n",
  

• 	addr, ret);
  

• return ret;

+}

+static int nl8048_init(struct nl8048_panel *lcd) +{

• static const struct {

• unsigned char addr;
  

• unsigned char data;
  

• } nl8048_init_seq[] = {

• {   3, 0x01 }, {   0, 0x00 }, {   1, 0x01 }, {   4, 0x00 },
  

• {   5, 0x14 }, {   6, 0x24 }, {  16, 0xd7 }, {  17, 0x00 },
  

• {  18, 0x00 }, {  19, 0x55 }, {  20, 0x01 }, {  21, 0x70 },
  

• {  22, 0x1e }, {  23, 0x25 }, {  24, 0x25 }, {  25, 0x02 },
  

• {  26, 0x02 }, {  27, 0xa0 }, {  32, 0x2f }, {  33, 0x0f },
  

• {  34, 0x0f }, {  35, 0x0f }, {  36, 0x0f }, {  37, 0x0f },
  

• {  38, 0x0f }, {  39, 0x00 }, {  40, 0x02 }, {  41, 0x02 },
  

• {  42, 0x02 }, {  43, 0x0f }, {  44, 0x0f }, {  45, 0x0f },
  

• {  46, 0x0f }, {  47, 0x0f }, {  48, 0x0f }, {  49, 0x0f },
  

• {  50, 0x00 }, {  51, 0x02 }, {  52, 0x02 }, {  53, 0x02 },
  

• {  80, 0x0c }, {  83, 0x42 }, {  84, 0x42 }, {  85, 0x41 },
  

• {  86, 0x14 }, {  89, 0x88 }, {  90, 0x01 }, {  91, 0x00 },
  

• {  92, 0x02 }, {  93, 0x0c }, {  94, 0x1c }, {  95, 0x27 },
  

• {  98, 0x49 }, {  99, 0x27 }, { 102, 0x76 }, { 103, 0x27 },
  

• { 112, 0x01 }, { 113, 0x0e }, { 114, 0x02 }, { 115, 0x0c },
  

• { 118, 0x0c }, { 121, 0x30 }, { 130, 0x00 }, { 131, 0x00 },
  

• { 132, 0xfc }, { 134, 0x00 }, { 136, 0x00 }, { 138, 0x00 },
  

• { 139, 0x00 }, { 140, 0x00 }, { 141, 0xfc }, { 143, 0x00 },
  

• { 145, 0x00 }, { 147, 0x00 }, { 148, 0x00 }, { 149, 0x00 },
  

• { 150, 0xfc }, { 152, 0x00 }, { 154, 0x00 }, { 156, 0x00 },
  

• { 157, 0x00 },
  

• };
• unsigned int i;
• int ret;
• for (i = 0; i < ARRAY_SIZE(nl8048_init_seq); ++i) {

• ret = nl8048_write(lcd, nl8048_init_seq[i].addr,
  

• 		   nl8048_init_seq[i].data);
  

• if (ret < 0)
  

• 	return ret;
  

• }
• udelay(20);
• return nl8048_write(lcd, 2, 0x00);

+}

+static int nl8048_disable(struct drm_panel *panel) +{

• struct nl8048_panel *lcd = to_nl8048_device(panel);
• gpiod_set_value_cansleep(lcd->reset_gpio, 0);
• return 0;

+}

+static int nl8048_enable(struct drm_panel *panel) +{

• struct nl8048_panel *lcd = to_nl8048_device(panel);
• gpiod_set_value_cansleep(lcd->reset_gpio, 1);
• return 0;

+}

+static const struct drm_display_mode nl8048_mode = {

• /* NEC PIX Clock Ratings MIN:21.8MHz TYP:23.8MHz MAX:25.7MHz */
• .clock = 23800,
• .hdisplay = 800,
• .hsync_start = 800 + 6,
• .hsync_end = 800 + 6 + 1,
• .htotal = 800 + 6 + 1 + 4,
• .vdisplay = 480,
• .vsync_start = 480 + 3,
• .vsync_end = 480 + 3 + 1,
• .vtotal = 480 + 3 + 1 + 4,
• .vrefresh = 60,
• .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
• .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
• .width_mm = 89,
• .height_mm = 53,

+};

+static int nl8048_get_modes(struct drm_panel *panel) +{

• struct drm_connector *connector = panel->connector;
• struct drm_display_mode *mode;
• mode = drm_mode_duplicate(panel->drm, &nl8048_mode);
• if (!mode)

• return -ENOMEM;
  

• drm_mode_set_name(mode);
• drm_mode_probed_add(connector, mode);
• connector->display_info.width_mm = nl8048_mode.width_mm;
• connector->display_info.height_mm = nl8048_mode.height_mm;
• connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH

• 			  | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
  

• 			  | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
  

• return 1;

+}

+static const struct drm_panel_funcs nl8048_funcs = {

• .disable = nl8048_disable,
• .enable = nl8048_enable,
• .get_modes = nl8048_get_modes,

+};

+static int __maybe_unused nl8048_suspend(struct device *dev) +{

• struct nl8048_panel *lcd = dev_get_drvdata(dev);
• nl8048_write(lcd, 2, 0x01);
• msleep(40);
• return 0;

+}

+static int __maybe_unused nl8048_resume(struct device *dev) +{

• struct nl8048_panel *lcd = dev_get_drvdata(dev);
• /* Reinitialize the panel. */
• spi_setup(lcd->spi);
• nl8048_write(lcd, 2, 0x00);
• nl8048_init(lcd);
• return 0;

+}

+static SIMPLE_DEV_PM_OPS(nl8048_pm_ops, nl8048_suspend, nl8048_resume);

+static int nl8048_probe(struct spi_device *spi) +{

• struct nl8048_panel *lcd;
• int ret;
• lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL);
• if (lcd == NULL)

• return -ENOMEM;
  

• spi_set_drvdata(spi, lcd);
• lcd->spi = spi;
• lcd->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
• if (IS_ERR(lcd->reset_gpio)) {

• dev_err(&spi->dev, "failed to parse reset gpio\n");
  

• return PTR_ERR(lcd->reset_gpio);
  

• }
• spi->mode = SPI_MODE_0;
• spi->bits_per_word = 32;
• ret = spi_setup(spi);
• if (ret < 0) {

• dev_err(&spi->dev, "failed to setup SPI: %d\n", ret);
  

• return ret;
  

• }
• ret = nl8048_init(lcd);
• if (ret < 0)

• return ret;
  

• drm_panel_init(&lcd->panel);
• lcd->panel.dev = &lcd->spi->dev;
• lcd->panel.funcs = &nl8048_funcs;
• return drm_panel_add(&lcd->panel);

+}

+static int nl8048_remove(struct spi_device *spi) +{

• struct nl8048_panel *lcd = spi_get_drvdata(spi);
• drm_panel_remove(&lcd->panel);
• drm_panel_disable(&lcd->panel);

We are missing a drm_panel_unprepare() here. This is required by the core - but not by this driver. (At least with the patch I have in the works).

Then again:

$ git grep -C 3 drm_panel_remove

tell a story that many users of drm_panel_remove do MOT call drm_panel_disable() / drm_panel_unprepare() after drm_panel_remove()

Maybe we should just add it to the core so disable/unprepare is done by the core when a panel is removed?

I think that's a good idea. I'll submit a v3 with drm_panel_unprepare() added, and moving that to the core could then be done through a tree-wide conversion.

...

• return 0;

+}

+static const struct of_device_id nl8048_of_match[] = {

• { .compatible = "nec,nl8048hl11", },
• { /* sentinel */ },

+};

+MODULE_DEVICE_TABLE(of, nl8048_of_match);

+static struct spi_driver nl8048_driver = {

• .probe = nl8048_probe,
• .remove = nl8048_remove,
• .driver = {

• .name	= "panel-nec-nl8048hl11",
  

• .pm	= &nl8048_pm_ops,
  

• .of_match_table = nl8048_of_match,
  

• },

+};

+module_spi_driver(nl8048_driver);

+MODULE_ALIAS("spi:nec,nl8048hl11"); +MODULE_AUTHOR("Erik Gilling konkers@android.com"); +MODULE_DESCRIPTION("NEC-NL8048HL11 Driver"); +MODULE_LICENSE("GPL");