Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 2/4] drm/tinydrm: add support for LEGO MINDSTORMS EV3 LCD | From | Noralf Trønnes <> | Date | Wed, 2 Aug 2017 15:03:03 +0200 |
| |
Den 02.08.2017 00.11, skrev David Lechner: > LEGO MINDSTORMS EV3 has an LCD with a ST7586 controller. This adds a new > module for the ST7586 controller with parameters for the EV3 LCD display. > > Signed-off-by: David Lechner <david@lechnology.com> > --- > .../devicetree/bindings/display/st7586.txt | 26 + > drivers/gpu/drm/tinydrm/Kconfig | 10 + > drivers/gpu/drm/tinydrm/Makefile | 1 + > drivers/gpu/drm/tinydrm/st7586.c | 579 +++++++++++++++++++++ > include/drm/tinydrm/st7586.h | 34 ++
Please add a MAINTAINERS entry. That way you'll be notified on changes.
> 5 files changed, 650 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/st7586.txt > create mode 100644 drivers/gpu/drm/tinydrm/st7586.c > create mode 100644 include/drm/tinydrm/st7586.h > > diff --git a/Documentation/devicetree/bindings/display/st7586.txt b/Documentation/devicetree/bindings/display/st7586.txt > new file mode 100644 > index 0000000..acf784aa > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/st7586.txt > @@ -0,0 +1,26 @@ > +Sitronix ST7586 display panel > + > +Required properties: > +- compatible: "lego,ev3-lcd". > + > +The node for this driver must be a child node of a SPI controller, hence > +all mandatory properties described in ../spi/spi-bus.txt must be specified. > + > +Optional properties: > +- dc-gpios: D/C pin. The presence/absence of this GPIO determines > + the panel interface mode (IM[3:0] pins): > + - present: IM=x110 4-wire 8-bit data serial interface > + - absent: IM=x101 3-wire 9-bit data serial interface
This doesn't match the ST7586 datasheet I have, the pins are called IF[3:1] and the values are different.
> +- reset-gpios: Reset pin > +- power-supply: A regulator node for the supply voltage. > +- backlight: phandle of the backlight device attached to the panel > +- rotation: panel rotation in degrees counter clockwise (0,90,180,270) > + > +Example: > + display@0{ > + compatible = "lego,ev3-lcd"; > + reg = <0>; > + spi-max-frequency = <10000000>; > + dc-gpios = <&gpio 43 GPIO_ACTIVE_HIGH>; > + reset-gpios = <&gpio 80 GPIO_ACTIVE_HIGH>; > + }; > diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig > index f17c3ca..2e790e7 100644 > --- a/drivers/gpu/drm/tinydrm/Kconfig > +++ b/drivers/gpu/drm/tinydrm/Kconfig > @@ -32,3 +32,13 @@ config TINYDRM_REPAPER > 2.71" TFT EPD Panel (E2271CS021) > > If M is selected the module will be called repaper. > + > +config TINYDRM_ST7586 > + tristate "DRM support for Sitronix ST7586 display panels" > + depends on DRM_TINYDRM && SPI > + select TINYDRM_MIPI_DBI > + help > + DRM driver for the following Sitronix ST7586 panels: > + * LEGO MINDSTORMS EV3 > + > + If M is selected the module will be called st7586. > diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile > index 95bb4d4..0c184bd 100644 > --- a/drivers/gpu/drm/tinydrm/Makefile > +++ b/drivers/gpu/drm/tinydrm/Makefile > @@ -6,3 +6,4 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI) += mipi-dbi.o > # Displays > obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o > obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o > +obj-$(CONFIG_TINYDRM_ST7586) += st7586.o > diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c > new file mode 100644 > index 0000000..6093021 > --- /dev/null > +++ b/drivers/gpu/drm/tinydrm/st7586.c > @@ -0,0 +1,579 @@ > +/* > + * DRM driver for Sitronix ST7586 panels > + * > + * Copyright 2017 David Lechner <david@lechnology.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/delay.h> > +#include <linux/dma-buf.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > +#include <linux/property.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > +#include <video/mipi_display.h> > + > +#include <drm/tinydrm/mipi-dbi.h> > +#include <drm/tinydrm/st7586.h> > +#include <drm/tinydrm/tinydrm-helpers.h> > + > +static inline u8 st7586_rgb565_to_gray2(u16 rgb) > +{ > + u8 r = (rgb & 0xf800) >> 11; > + u8 g = (rgb & 0x07e0) >> 5; > + u8 b = rgb & 0x001f; > + /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */ > + /* g is already * 2 because it is 6-bit */ > + u8 gray5 = (3 * r + 3 * g + b) / 10; > + > + return gray5 >> 3; > +} > + > +static void st7586_from_rgb565(u8 *dst, void *vaddr, > + struct drm_framebuffer *fb, > + struct drm_clip_rect *clip) > +{ > + size_t len; > + unsigned int x, y; > + u16 *src, *buf; > + u8 val; > + > + /* 3 pixels per byte, so grow clip to nearest multiple of 3 */ > + clip->x1 = clip->x1 / 3 * 3; > + clip->x2 = (clip->x2 + 2) / 3 * 3; > + len = (clip->x2 - clip->x1) * sizeof(u16); > + > + buf = kmalloc(len, GFP_KERNEL); > + if (!buf) > + return; > + > + for (y = clip->y1; y < clip->y2; y++) { > + src = vaddr + (y * fb->pitches[0]); > + src += clip->x1; > + memcpy(buf, src, len); > + src = buf; > + for (x = clip->x1; x < clip->x2; x += 3) { > + val = st7586_rgb565_to_gray2(*src++) << 6; > + if (val & 0xC0) > + val |= 0x20; > + val |= st7586_rgb565_to_gray2(*src++) << 3; > + if (val & 0x18) > + val |= 0x04; > + val |= st7586_rgb565_to_gray2(*src++); > + *dst++ = ~val; > + } > + } > + > + /* now adjust the clip so it applies to dst */ > + clip->x1 /= 3; > + clip->x2 /= 3; > + > + kfree(buf); > +} > + > +static inline u8 st7586_xrgb8888_to_gray2(u32 rgb) > +{ > + u8 r = (rgb & 0x00ff0000) >> 16; > + u8 g = (rgb & 0x0000ff00) >> 8; > + u8 b = rgb & 0x000000ff; > + /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */ > + u8 gray8 = (3 * r + 6 * g + b) / 10; > + > + return gray8 >> 6; > +} > + > +static void st7586_from_xrgb8888(u8 *dst, void *vaddr, > + struct drm_framebuffer *fb, > + struct drm_clip_rect *clip) > +{ > + size_t len; > + unsigned int x, y; > + u32 *src, *buf; > + u8 val; > + > + /* 3 pixels per byte, so grow clip to nearest multiple of 3 */ > + clip->x1 = clip->x1 / 3 * 3; > + clip->x2 = (clip->x2 + 2) / 3 * 3; > + len = (clip->x2 - clip->x1) * sizeof(u32); > + > + buf = kmalloc(len, GFP_KERNEL); > + if (!buf) > + return; > + > + for (y = clip->y1; y < clip->y2; y++) { > + src = vaddr + (y * fb->pitches[0]); > + src += clip->x1; > + memcpy(buf, src, len); > + src = buf; > + for (x = clip->x1; x < clip->x2; x += 3) { > + val = st7586_xrgb8888_to_gray2(*src++) << 6; > + if (val & 0xC0) > + val |= 0x20; > + val |= st7586_xrgb8888_to_gray2(*src++) << 3; > + if (val & 0x18) > + val |= 0x04; > + val |= st7586_xrgb8888_to_gray2(*src++); > + *dst++ = ~val; > + } > + } > + > + /* now adjust the clip so it applies to dst */ > + clip->x1 /= 3; > + clip->x2 /= 3; > + > + kfree(buf); > +}
Please use tinydrm_xrgb8888_to_gray8().
> + > +static int st7586_mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
You can shorten your st7586_mipi_dbi_* to just st7586_*
> + struct drm_clip_rect *clip) > +{ > + struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); > + struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; > + struct drm_format_name_buf format_name; > + void *src = cma_obj->vaddr; > + int ret = 0; > + > + if (import_attach) { > + ret = dma_buf_begin_cpu_access(import_attach->dmabuf, > + DMA_FROM_DEVICE); > + if (ret) > + return ret; > + } > + > + switch (fb->format->format) { > + case DRM_FORMAT_RGB565: > + st7586_from_rgb565(dst, src, fb, clip); > + break; > + case DRM_FORMAT_XRGB8888: > + st7586_from_xrgb8888(dst, src, fb, clip); > + break; > + default: > + dev_err_once(fb->dev->dev, "Format is not supported: %s\n", > + drm_get_format_name(fb->format->format, > + &format_name)); > + ret = -EINVAL; > + break; > + } > + > + if (import_attach) > + ret = dma_buf_end_cpu_access(import_attach->dmabuf, > + DMA_FROM_DEVICE); > + return ret; > +} > + > +static int st7586_mipi_dbi_fb_dirty(struct drm_framebuffer *fb, > + struct drm_file *file_priv, > + unsigned int flags, unsigned int color, > + struct drm_clip_rect *clips, > + unsigned int num_clips) > +{ > + struct tinydrm_device *tdev = fb->dev->dev_private; > + struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); > + struct drm_clip_rect clip; > + int ret = 0; > + > + mutex_lock(&tdev->dirty_lock); > + > + if (!mipi->enabled) > + goto out_unlock; > + > + /* fbdev can flush even when we're not interested */ > + if (tdev->pipe.plane.fb != fb) > + goto out_unlock; > + > + tinydrm_merge_clips(&clip, clips, num_clips, flags, fb->width, > + fb->height); > + > + DRM_DEBUG("Flushing [FB:%d] x1=%u, x2=%u, y1=%u, y2=%u\n", fb->base.id, > + clip.x1, clip.x2, clip.y1, clip.y2); > + > + ret = st7586_mipi_dbi_buf_copy(mipi->tx_buf, fb, &clip); > + if (ret) > + goto out_unlock; > + > + /* NB: st7586_mipi_dbi_buf_copy() modifies clip */ > + > + mipi_dbi_command(mipi, MIPI_DCS_SET_COLUMN_ADDRESS, > + (clip.x1 >> 8) & 0xFF, clip.x1 & 0xFF, > + (clip.x2 >> 8) & 0xFF, (clip.x2 - 1) & 0xFF); > + mipi_dbi_command(mipi, MIPI_DCS_SET_PAGE_ADDRESS, > + (clip.y1 >> 8) & 0xFF, clip.y1 & 0xFF, > + (clip.y2 >> 8) & 0xFF, (clip.y2 - 1) & 0xFF); > + > + ret = mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, > + (u8 *)mipi->tx_buf, > + (clip.x2 - clip.x1) * (clip.y2 - clip.y1)); > + > +out_unlock: > + mutex_unlock(&tdev->dirty_lock); > + > + if (ret) > + dev_err_once(fb->dev->dev, "Failed to update display %d\n", > + ret); > + > + return ret; > +} > + > +static const struct drm_framebuffer_funcs st7586_mipi_dbi_fb_funcs = { > + .destroy = drm_fb_cma_destroy, > + .create_handle = drm_fb_cma_create_handle, > + .dirty = st7586_mipi_dbi_fb_dirty, > +}; > + > +void st7586_mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, > + struct drm_crtc_state *crtc_state) > +{ > + struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); > + struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); > + struct drm_framebuffer *fb = pipe->plane.fb; > + > + DRM_DEBUG_KMS("\n"); > + You should use this function to enable the regulator and init the controller. Don't look at mi0283qt it should have been fixed.
> + mipi->enabled = true; > + if (fb)
I don't think it's possible to enable the pipe without a framebuffer, but I wasn't able find out. drm is complex...
> + fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0); > + > + tinydrm_enable_backlight(mipi->backlight); > +} > + > +static void st7586_mipi_dbi_blank(struct mipi_dbi *mipi) > +{ > + struct drm_device *drm = mipi->tinydrm.drm; > + u16 height = drm->mode_config.min_height; > + u16 width = (drm->mode_config.min_width + 2) / 3; > + size_t len = width * height; > + > + memset(mipi->tx_buf, 0, len); > + > + mipi_dbi_command(mipi, MIPI_DCS_SET_COLUMN_ADDRESS, 0, 0, > + (width >> 8) & 0xFF, (width - 1) & 0xFF); > + mipi_dbi_command(mipi, MIPI_DCS_SET_PAGE_ADDRESS, 0, 0, > + (height >> 8) & 0xFF, (height - 1) & 0xFF); > + mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, > + (u8 *)mipi->tx_buf, len); > +} > + > +static void st7586_mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe) > +{ > + struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); > + struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); > + > + DRM_DEBUG_KMS("\n"); > + > + mipi->enabled = false; > + > + if (mipi->backlight) > + tinydrm_disable_backlight(mipi->backlight); > + else > + st7586_mipi_dbi_blank(mipi);
And disable the regulator here.
> +} > + > +static const u32 st7586_mipi_dbi_formats[] = { > + DRM_FORMAT_RGB565, > + DRM_FORMAT_XRGB8888,
Why 2 emulation formats? I chose DRM_FORMAT_XRGB8888 since I expected everything to support it. Please use only that.
> +}; > + > +static int st7586_mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, > + const struct drm_simple_display_pipe_funcs *pipe_funcs, > + struct drm_driver *driver, const struct drm_display_mode *mode, > + unsigned int rotation) > +{ > + size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay; > + struct tinydrm_device *tdev = &mipi->tinydrm; > + int ret; > + > + if (!mipi->command) > + return -EINVAL;
You know this is set after calling mipi_dbi_spi_init(), so you don't have to check.
> + > + mutex_init(&mipi->cmdlock); > + > + mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL); > + if (!mipi->tx_buf) > + return -ENOMEM; > + > + ret = devm_tinydrm_init(dev, tdev, &st7586_mipi_dbi_fb_funcs, driver); > + if (ret) > + return ret; > + > + ret = tinydrm_display_pipe_init(tdev, pipe_funcs, > + DRM_MODE_CONNECTOR_VIRTUAL, > + st7586_mipi_dbi_formats, > + ARRAY_SIZE(st7586_mipi_dbi_formats), > + mode, rotation); > + if (ret) > + return ret; > + > + tdev->drm->mode_config.preferred_depth = 16; > + mipi->rotation = rotation; > + > + drm_mode_config_reset(tdev->drm); > + > + DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", > + tdev->drm->mode_config.preferred_depth, rotation); > + > + return 0; > +} > + > +static int st7586_init(struct mipi_dbi *mipi) > +{ > + struct tinydrm_device *tdev = &mipi->tinydrm; > + struct device *dev = tdev->drm->dev; > + u8 addr_mode; > + int ret; > + > + DRM_DEBUG_KMS("\n"); > + > + ret = regulator_enable(mipi->regulator); > + if (ret) { > + dev_err(dev, "Failed to enable regulator %d\n", ret); > + return ret; > + } > + > + /* Avoid flicker by skipping setup if the bootloader has done it */ > + if (mipi_dbi_display_is_on(mipi))
Looking at the datasheet it seems that it's not possible to read from the controller via SPI?
> + return 0; > + > + mipi_dbi_hw_reset(mipi); > + ret = mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f); > + if (ret) { > + dev_err(dev, "Error sending command %d\n", ret); > + regulator_disable(mipi->regulator); > + return ret; > + } > + > + mipi_dbi_command(mipi, ST7586_OTP_RW_CTRL, 0x00); > + > + msleep(10); > + > + mipi_dbi_command(mipi, ST7586_OTP_READ); > + > + msleep(20); > + > + mipi_dbi_command(mipi, ST7586_OTP_CTRL_OUT); > + mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE); > + mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF); > + > + msleep(50); > + > + mipi_dbi_command(mipi, ST7586_SET_VOP_OFFSET, 0x00); > + mipi_dbi_command(mipi, ST7586_SET_VOP, 0xe3, 0x00); > + mipi_dbi_command(mipi, ST7586_SET_BIAS_SYSTEM, 0x02); > + mipi_dbi_command(mipi, ST7586_SET_BOOST_LEVEL, 0x04); > + mipi_dbi_command(mipi, ST7586_ENABLE_ANALOG, 0x1d); > + mipi_dbi_command(mipi, ST7586_SET_NLINE_INV, 0x00); > + mipi_dbi_command(mipi, ST7586_DISP_MODE_GRAY); > + mipi_dbi_command(mipi, ST7586_ENABLE_DDRAM, 0x02); > + > + switch (mipi->rotation) { > + default: > + addr_mode = 0x00; > + break; > + case 90: > + addr_mode = ST7586_DISP_CTRL_MY; > + break; > + case 180: > + addr_mode = ST7586_DISP_CTRL_MX | ST7586_DISP_CTRL_MY; > + break; > + case 270: > + addr_mode = ST7586_DISP_CTRL_MX; > + break; > + } > + mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); > + > + mipi_dbi_command(mipi, ST7586_SET_DISP_DUTY, 0x7f); > + mipi_dbi_command(mipi, ST7586_SET_PART_DISP, 0xa0); > + mipi_dbi_command(mipi, MIPI_DCS_SET_PARTIAL_AREA, 0x00, 0x00, 0x00, 0x77); > + mipi_dbi_command(mipi, MIPI_DCS_EXIT_INVERT_MODE); > + > + msleep(100); > + > + mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON); > + > + return 0; > +} > + > +static void st7586_fini(void *data) > +{ > + struct mipi_dbi *mipi = data; > + > + DRM_DEBUG_KMS("\n"); > + regulator_disable(mipi->regulator); > +} > + > +static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = { > + .enable = st7586_mipi_dbi_pipe_enable, > + .disable = st7586_mipi_dbi_pipe_disable, > + .update = tinydrm_display_pipe_update, > + .prepare_fb = tinydrm_display_pipe_prepare_fb, > +}; > + > +static const struct drm_display_mode st7586_mode = { > + TINYDRM_MODE(178, 128, 37, 27), > +}; > + > +DEFINE_DRM_GEM_CMA_FOPS(st7586_fops); > + > +static struct drm_driver st7586_driver = { > + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | > + DRIVER_ATOMIC, > + .fops = &st7586_fops, > + TINYDRM_GEM_DRIVER_OPS, > + .lastclose = tinydrm_lastclose, > + .debugfs_init = mipi_dbi_debugfs_init, > + .name = "st7586", > + .desc = "Sitronix ST7586", > + .date = "20170801", > + .major = 1, > + .minor = 0, > +}; > + > +static const struct of_device_id st7586_of_match[] = { > + { .compatible = "lego,ev3-lcd" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, st7586_of_match); > + > +static const struct spi_device_id st7586_id[] = { > + { "ev3-lcd", 0 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(spi, st7586_id); > + > +static int st7586_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + struct tinydrm_device *tdev; > + struct mipi_dbi *mipi; > + struct gpio_desc *dc; > + u32 rotation = 0; > + int ret; > + > + mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL); > + if (!mipi) > + return -ENOMEM; > + > + mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(mipi->reset)) { > + dev_err(dev, "Failed to get gpio 'reset'\n"); > + return PTR_ERR(mipi->reset); > + } > + > + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); > + if (IS_ERR(dc)) { > + dev_err(dev, "Failed to get gpio 'dc'\n"); > + return PTR_ERR(dc); > + } > + > + mipi->regulator = devm_regulator_get(dev, "power"); > + if (IS_ERR(mipi->regulator)) > + return PTR_ERR(mipi->regulator); > + > + mipi->backlight = tinydrm_of_find_backlight(dev); > + if (IS_ERR(mipi->backlight)) > + return PTR_ERR(mipi->backlight); > + > + device_property_read_u32(dev, "rotation", &rotation); > + > + ret = mipi_dbi_spi_init(spi, mipi, dc); > + if (ret) > + return ret;
Since you are using mipi_dbi_debugfs_init and the controller can't be read, disable reading:
mipi->read_commands = NULL;
> + > + /* > + * we are using 8-bit data, so we are not actually swapping anything, > + * but setting mipi->swap_bytes makes mipi_dbi_typec3_command() do the > + * right thing and not use 16-bit transfers (which results in swapped > + * bytes on little-endian systems and causes out of order data to be > + * sent to the display). > + */ > + mipi->swap_bytes = true; > + > + ret = st7586_mipi_dbi_init(&spi->dev, mipi, &st7586_pipe_funcs, > + &st7586_driver, &st7586_mode, rotation); > + if (ret) > + return ret; > + > + ret = st7586_init(mipi);
This isn't necessary when you use .enable/.disable to control power/init, and you can drop the devm_add_action.
> + if (ret) > + return ret; > + > + /* use devres to fini after drm unregister (drv->remove is before) */ > + ret = devm_add_action(dev, st7586_fini, mipi); > + if (ret) { > + st7586_fini(mipi); > + return ret; > + } > + > + tdev = &mipi->tinydrm; > + > + ret = devm_tinydrm_register(tdev); > + if (ret) > + return ret; > + > + spi_set_drvdata(spi, mipi); > + > + DRM_DEBUG_DRIVER("Initialized %s:%s @%uMHz on minor %d\n", > + tdev->drm->driver->name, dev_name(dev), > + spi->max_speed_hz / 1000000, > + tdev->drm->primary->index); > + > + return 0; > +} > + > +static void st7586_shutdown(struct spi_device *spi) > +{ > + struct mipi_dbi *mipi = spi_get_drvdata(spi); > + > + tinydrm_shutdown(&mipi->tinydrm); > +} > + > +static int __maybe_unused st7586_pm_suspend(struct device *dev) > +{ > + struct mipi_dbi *mipi = dev_get_drvdata(dev); > + int ret; > + > + ret = tinydrm_suspend(&mipi->tinydrm); > + if (ret) > + return ret; > + > + st7586_fini(mipi); > +
With .enable/.disable controlling power, tinydrm_suspend() is enough.
> + return 0; > +} > + > +static int __maybe_unused st7586_pm_resume(struct device *dev) > +{ > + struct mipi_dbi *mipi = dev_get_drvdata(dev); > + int ret; > + > + ret = st7586_init(mipi); > + if (ret) > + return ret; > +
Same goes here.
> + return tinydrm_resume(&mipi->tinydrm); > +} > + > +static const struct dev_pm_ops st7586_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(st7586_pm_suspend, st7586_pm_resume) > +}; > + > +static struct spi_driver st7586_spi_driver = { > + .driver = { > + .name = "st7586", > + .owner = THIS_MODULE, > + .of_match_table = st7586_of_match, > + .pm = &st7586_pm_ops, > + }, > + .id_table = st7586_id, > + .probe = st7586_probe, > + .shutdown = st7586_shutdown, > +}; > +module_spi_driver(st7586_spi_driver); > + > +MODULE_DESCRIPTION("Sitronix ST7586 DRM driver"); > +MODULE_AUTHOR("David Lechner <david@lechnology.com>"); > +MODULE_LICENSE("GPL"); > diff --git a/include/drm/tinydrm/st7586.h b/include/drm/tinydrm/st7586.h > new file mode 100644 > index 0000000..18fb56b > --- /dev/null > +++ b/include/drm/tinydrm/st7586.h
Please put this in drivers/drm/tinydrm, or you can even put the defines in the driver if you want, it's quite short. ili9341.h shouldn't have ended up in include/drm/tinydrm.
Noralf.
> @@ -0,0 +1,34 @@ > +/* > + * ST7586 LCD controller > + * > + * Copyright (C) 2017 David Lechner <david@lechnology.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#ifndef __LINUX_ST7856_H > +#define __LINUX_ST7856_H > + > +#define ST7586_DISP_MODE_GRAY 0x38 > +#define ST7586_DISP_MODE_MONO 0x39 > +#define ST7586_ENABLE_DDRAM 0x3a > +#define ST7586_SET_DISP_DUTY 0xb0 > +#define ST7586_SET_PART_DISP 0xb4 > +#define ST7586_SET_NLINE_INV 0xb5 > +#define ST7586_SET_VOP 0xc0 > +#define ST7586_SET_BIAS_SYSTEM 0xc3 > +#define ST7586_SET_BOOST_LEVEL 0xc4 > +#define ST7586_SET_VOP_OFFSET 0xc7 > +#define ST7586_ENABLE_ANALOG 0xd0 > +#define ST7586_AUTO_READ_CTRL 0xd7 > +#define ST7586_OTP_RW_CTRL 0xe0 > +#define ST7586_OTP_CTRL_OUT 0xe1 > +#define ST7586_OTP_READ 0xe3 > + > +#define ST7586_DISP_CTRL_MX BIT(6) > +#define ST7586_DISP_CTRL_MY BIT(7) > + > +#endif /* __LINUX_ST7856_H */
| |