Fix a few things that came up during the tinydrm review but wasn't addressed at the time.
Noralf.
Changes since version 2: - s/mipi_dbi_por_conditional/mipi_dbi_poweron_reset_conditional/ - Add sleep after soft reset to comply with spec. - Remove reset msleep from mi0283qt following the previous change. - Use (ret == 1) to test for display is ON, no reset. - Add patch to change hard reset active time.
Changes since version 1: - Add mipi_dbi_enable_flush() instead of the pipe enable helper mipi_dbi_pipe_enable() that doesn't really fit anymore. - Add common poweron-reset functionality. - Use drm_mode_copy() instead of direct assignment
Noralf Trønnes (7): drm/tinydrm/mi0283qt: Use common include order drm/tinydrm/mi0283qt: Remove ili9341.h drm/tinydrm/mipi-dbi: Add mipi_dbi_enable_flush() drm/tinydrm/mipi-dbi: Add poweron-reset functions drm/tinydrm/mi0283qt: Let the display pipe handle power drm/tinydrm: Embed the mode in tinydrm_connector drm/tinydrm/mipi-dbi: Change reset active time
drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 34 ++++----- drivers/gpu/drm/tinydrm/ili9225.c | 6 +- drivers/gpu/drm/tinydrm/mi0283qt.c | 106 ++++++++++++---------------- drivers/gpu/drm/tinydrm/mipi-dbi.c | 101 ++++++++++++++++++++++---- drivers/gpu/drm/tinydrm/st7586.c | 15 ++-- drivers/gpu/drm/tinydrm/st7735r.c | 10 +-- include/drm/tinydrm/ili9341.h | 54 -------------- include/drm/tinydrm/mipi-dbi.h | 5 +- 8 files changed, 154 insertions(+), 177 deletions(-) delete mode 100644 include/drm/tinydrm/ili9341.h
Include linux headers before drm headers as it's commonly done.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: David Lechner david@lechnology.com --- drivers/gpu/drm/tinydrm/mi0283qt.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 674d407640be..45f02b6052b1 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -9,17 +9,18 @@ * (at your option) any later version. */
-#include <drm/drm_fb_helper.h> -#include <drm/drm_modeset_helper.h> -#include <drm/tinydrm/ili9341.h> -#include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h> #include <linux/delay.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 <drm/drm_fb_helper.h> +#include <drm/drm_modeset_helper.h> +#include <drm/tinydrm/ili9341.h> +#include <drm/tinydrm/mipi-dbi.h> +#include <drm/tinydrm/tinydrm-helpers.h> #include <video/mipi_display.h>
static int mi0283qt_init(struct mipi_dbi *mipi)
No need for a public header file for the command macros. Just include the necessary ones in the driver.
Also use the MIPI_DCS_PIXEL_FMT_16BIT macro.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: David Lechner david@lechnology.com --- drivers/gpu/drm/tinydrm/mi0283qt.c | 28 ++++++++++++++++++-- include/drm/tinydrm/ili9341.h | 54 -------------------------------------- 2 files changed, 26 insertions(+), 56 deletions(-) delete mode 100644 include/drm/tinydrm/ili9341.h
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 45f02b6052b1..c69a4d958f24 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -18,11 +18,35 @@
#include <drm/drm_fb_helper.h> #include <drm/drm_modeset_helper.h> -#include <drm/tinydrm/ili9341.h> #include <drm/tinydrm/mipi-dbi.h> #include <drm/tinydrm/tinydrm-helpers.h> #include <video/mipi_display.h>
+#define ILI9341_FRMCTR1 0xb1 +#define ILI9341_DISCTRL 0xb6 +#define ILI9341_ETMOD 0xb7 + +#define ILI9341_PWCTRL1 0xc0 +#define ILI9341_PWCTRL2 0xc1 +#define ILI9341_VMCTRL1 0xc5 +#define ILI9341_VMCTRL2 0xc7 +#define ILI9341_PWCTRLA 0xcb +#define ILI9341_PWCTRLB 0xcf + +#define ILI9341_PGAMCTRL 0xe0 +#define ILI9341_NGAMCTRL 0xe1 +#define ILI9341_DTCTRLA 0xe8 +#define ILI9341_DTCTRLB 0xea +#define ILI9341_PWRSEQ 0xed + +#define ILI9341_EN3GAM 0xf2 +#define ILI9341_PUMPCTRL 0xf7 + +#define ILI9341_MADCTL_BGR BIT(3) +#define ILI9341_MADCTL_MV BIT(5) +#define ILI9341_MADCTL_MX BIT(6) +#define ILI9341_MADCTL_MY BIT(7) + static int mi0283qt_init(struct mipi_dbi *mipi) { struct tinydrm_device *tdev = &mipi->tinydrm; @@ -69,7 +93,7 @@ static int mi0283qt_init(struct mipi_dbi *mipi) mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0xbe);
/* Memory Access Control */ - mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, 0x55); + mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
switch (mipi->rotation) { default: diff --git a/include/drm/tinydrm/ili9341.h b/include/drm/tinydrm/ili9341.h deleted file mode 100644 index 807a09f43cad..000000000000 --- a/include/drm/tinydrm/ili9341.h +++ /dev/null @@ -1,54 +0,0 @@ -/* - * ILI9341 LCD controller - * - * Copyright 2016 Noralf Trønnes - * - * 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_ILI9341_H -#define __LINUX_ILI9341_H - -#define ILI9341_FRMCTR1 0xb1 -#define ILI9341_FRMCTR2 0xb2 -#define ILI9341_FRMCTR3 0xb3 -#define ILI9341_INVTR 0xb4 -#define ILI9341_PRCTR 0xb5 -#define ILI9341_DISCTRL 0xb6 -#define ILI9341_ETMOD 0xb7 - -#define ILI9341_PWCTRL1 0xc0 -#define ILI9341_PWCTRL2 0xc1 -#define ILI9341_VMCTRL1 0xc5 -#define ILI9341_VMCTRL2 0xc7 -#define ILI9341_PWCTRLA 0xcb -#define ILI9341_PWCTRLB 0xcf - -#define ILI9341_RDID1 0xda -#define ILI9341_RDID2 0xdb -#define ILI9341_RDID3 0xdc -#define ILI9341_RDID4 0xd3 - -#define ILI9341_PGAMCTRL 0xe0 -#define ILI9341_NGAMCTRL 0xe1 -#define ILI9341_DGAMCTRL1 0xe2 -#define ILI9341_DGAMCTRL2 0xe3 -#define ILI9341_DTCTRLA 0xe8 -#define ILI9341_DTCTRLB 0xea -#define ILI9341_PWRSEQ 0xed - -#define ILI9341_EN3GAM 0xf2 -#define ILI9341_IFCTRL 0xf6 -#define ILI9341_PUMPCTRL 0xf7 - -#define ILI9341_MADCTL_MH BIT(2) -#define ILI9341_MADCTL_BGR BIT(3) -#define ILI9341_MADCTL_ML BIT(4) -#define ILI9341_MADCTL_MV BIT(5) -#define ILI9341_MADCTL_MX BIT(6) -#define ILI9341_MADCTL_MY BIT(7) - -#endif /* __LINUX_ILI9341_H */
Add and use a function for enabling, flushing and turning on backlight.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: David Lechner david@lechnology.com --- drivers/gpu/drm/tinydrm/ili9225.c | 6 +----- drivers/gpu/drm/tinydrm/mipi-dbi.c | 20 ++++++++++++++++++++ drivers/gpu/drm/tinydrm/st7586.c | 6 +----- drivers/gpu/drm/tinydrm/st7735r.c | 2 +- include/drm/tinydrm/mipi-dbi.h | 1 + 5 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index c0cf49849302..a0759502b81a 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -180,7 +180,6 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe, { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); - struct drm_framebuffer *fb = pipe->plane.fb; struct device *dev = tdev->drm->dev; int ret; u8 am_id; @@ -269,10 +268,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
ili9225_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x1017);
- mipi->enabled = true; - - if (fb) - fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0); + mipi_dbi_enable_flush(mipi); }
static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe) diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index aa6b6ce56891..1c8ef0c4d6d4 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -270,6 +270,26 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = { .dirty = mipi_dbi_fb_dirty, };
+/** + * mipi_dbi_enable_flush - MIPI DBI enable helper + * @mipi: MIPI DBI structure + * + * This function sets &mipi_dbi->enabled, flushes the whole framebuffer and + * enables the backlight. Drivers can use this in their + * &drm_simple_display_pipe_funcs->enable callback. + */ +void mipi_dbi_enable_flush(struct mipi_dbi *mipi) +{ + struct drm_framebuffer *fb = mipi->tinydrm.pipe.plane.fb; + + mipi->enabled = true; + if (fb) + fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0); + + tinydrm_enable_backlight(mipi->backlight); +} +EXPORT_SYMBOL(mipi_dbi_enable_flush); + /** * mipi_dbi_pipe_enable - MIPI DBI pipe enable helper * @pipe: Display pipe diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 5aebfceb740e..9fd4423c8e70 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -179,7 +179,6 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); - struct drm_framebuffer *fb = pipe->plane.fb; struct device *dev = tdev->drm->dev; int ret; u8 addr_mode; @@ -241,10 +240,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
- mipi->enabled = true; - - if (fb) - fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0); + mipi_dbi_enable_flush(mipi); }
static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe) diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index 98ff447f40b4..1f38e15da676 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c @@ -102,7 +102,7 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
msleep(20);
- mipi_dbi_pipe_enable(pipe, crtc_state); + mipi_dbi_enable_flush(mipi); }
static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = { diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 5d0e82b36eaf..6441d9a9161a 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -67,6 +67,7 @@ int 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); +void mipi_dbi_enable_flush(struct mipi_dbi *mipi); void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state); void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe);
Split out common poweron-reset functionality.
Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/mi0283qt.c | 22 ++---------- drivers/gpu/drm/tinydrm/mipi-dbi.c | 73 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/tinydrm/st7586.c | 9 ++--- drivers/gpu/drm/tinydrm/st7735r.c | 8 ++--- include/drm/tinydrm/mipi-dbi.h | 2 ++ 5 files changed, 83 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index c69a4d958f24..1617405faed4 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -49,33 +49,17 @@
static int mi0283qt_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) { - DRM_DEV_ERROR(dev, "Failed to enable regulator %d\n", ret); + ret = mipi_dbi_poweron_conditional_reset(mipi); + if (ret < 0) return ret; - } - - /* Avoid flicker by skipping setup if the bootloader has done it */ - if (mipi_dbi_display_is_on(mipi)) + if (ret == 1) return 0;
- mipi_dbi_hw_reset(mipi); - ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); - if (ret) { - DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); - regulator_disable(mipi->regulator); - return ret; - } - - msleep(20); - mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30); diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 1c8ef0c4d6d4..3e879d605ed3 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -463,6 +463,7 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi)
val &= ~DCS_POWER_MODE_RESERVED_MASK;
+ /* The poweron/reset value is 08h DCS_POWER_MODE_DISPLAY_NORMAL_MODE */ if (val != (DCS_POWER_MODE_DISPLAY | DCS_POWER_MODE_DISPLAY_NORMAL_MODE | DCS_POWER_MODE_SLEEP_MODE)) return false; @@ -473,6 +474,78 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi) } EXPORT_SYMBOL(mipi_dbi_display_is_on);
+static int mipi_dbi_poweron_reset_conditional(struct mipi_dbi *mipi, bool cond) +{ + struct device *dev = mipi->tinydrm.drm->dev; + int ret; + + if (mipi->regulator) { + ret = regulator_enable(mipi->regulator); + if (ret) { + DRM_DEV_ERROR(dev, "Failed to enable regulator (%d)\n", ret); + return ret; + } + } + + if (cond && mipi_dbi_display_is_on(mipi)) + return 1; + + mipi_dbi_hw_reset(mipi); + ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); + if (ret) { + DRM_DEV_ERROR(dev, "Failed to send reset command (%d)\n", ret); + if (mipi->regulator) + regulator_disable(mipi->regulator); + return ret; + } + + /* + * If we did a hw reset, we know the controller is in Sleep mode and + * per MIPI DSC spec should wait 5ms after soft reset. If we didn't, + * we assume worst case and wait 120ms. + */ + if (mipi->reset) + usleep_range(5000, 20000); + else + msleep(120); + + return 0; +} + +/** + * mipi_dbi_poweron_reset - MIPI DBI poweron and reset + * @mipi: MIPI DBI structure + * + * This function enables the regulator if used and does a hardware and software + * reset. + * + * Returns: + * Zero on success, or a negative error code. + */ +int mipi_dbi_poweron_reset(struct mipi_dbi *mipi) +{ + return mipi_dbi_poweron_reset_conditional(mipi, false); +} +EXPORT_SYMBOL(mipi_dbi_poweron_reset); + +/** + * mipi_dbi_poweron_conditional_reset - MIPI DBI poweron and conditional reset + * @mipi: MIPI DBI structure + * + * This function enables the regulator if used and if the display is off, it + * does a hardware and software reset. If mipi_dbi_display_is_on() determines + * that the display is on, no reset is performed. + * + * Returns: + * Zero if the controller was reset, 1 if the display was already on, or a + * negative error code. + */ +int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi) +{ + return mipi_dbi_poweron_reset_conditional(mipi, true); +} +EXPORT_SYMBOL(mipi_dbi_poweron_conditional_reset); + #if IS_ENABLED(CONFIG_SPI)
/** diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 9fd4423c8e70..a6396ef9cc4a 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -179,19 +179,16 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); - struct device *dev = tdev->drm->dev; int ret; u8 addr_mode;
DRM_DEBUG_KMS("\n");
- mipi_dbi_hw_reset(mipi); - ret = mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f); - if (ret) { - DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); + ret = mipi_dbi_poweron_reset(mipi); + if (ret) return; - }
+ mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f); mipi_dbi_command(mipi, ST7586_OTP_RW_CTRL, 0x00);
msleep(10); diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index 1f38e15da676..650257ad0193 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c @@ -46,13 +46,9 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
DRM_DEBUG_KMS("\n");
- mipi_dbi_hw_reset(mipi); - - ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); - if (ret) { - DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); + ret = mipi_dbi_poweron_reset(mipi); + if (ret) return; - }
msleep(150);
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 6441d9a9161a..795a4a2205bb 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -73,6 +73,8 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe); void mipi_dbi_hw_reset(struct mipi_dbi *mipi); bool mipi_dbi_display_is_on(struct mipi_dbi *mipi); +int mipi_dbi_poweron_reset(struct mipi_dbi *mipi); +int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi); u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len);
int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
On 01/10/2018 12:59 PM, Noralf Trønnes wrote:
Split out common poweron-reset functionality.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/tinydrm/mi0283qt.c | 22 ++---------- drivers/gpu/drm/tinydrm/mipi-dbi.c | 73 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/tinydrm/st7586.c | 9 ++--- drivers/gpu/drm/tinydrm/st7735r.c | 8 ++--- include/drm/tinydrm/mipi-dbi.h | 2 ++ 5 files changed, 83 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index c69a4d958f24..1617405faed4 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -49,33 +49,17 @@
static int mi0283qt_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) {
DRM_DEV_ERROR(dev, "Failed to enable regulator %d\n", ret);
- ret = mipi_dbi_poweron_conditional_reset(mipi);
- if (ret < 0) return ret;
- }
- /* Avoid flicker by skipping setup if the bootloader has done it */
It could be helpful to keep this comment.
- if (mipi_dbi_display_is_on(mipi))
- if (ret == 1) return 0;
mipi_dbi_hw_reset(mipi);
ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET);
if (ret) {
DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);regulator_disable(mipi->regulator);return ret;}
msleep(20);
mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30);
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 1c8ef0c4d6d4..3e879d605ed3 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -463,6 +463,7 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi)
val &= ~DCS_POWER_MODE_RESERVED_MASK;
- /* The poweron/reset value is 08h DCS_POWER_MODE_DISPLAY_NORMAL_MODE */ if (val != (DCS_POWER_MODE_DISPLAY | DCS_POWER_MODE_DISPLAY_NORMAL_MODE | DCS_POWER_MODE_SLEEP_MODE)) return false;
@@ -473,6 +474,78 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi) } EXPORT_SYMBOL(mipi_dbi_display_is_on);
+static int mipi_dbi_poweron_reset_conditional(struct mipi_dbi *mipi, bool cond) +{
- struct device *dev = mipi->tinydrm.drm->dev;
- int ret;
- if (mipi->regulator) {
ret = regulator_enable(mipi->regulator);if (ret) {DRM_DEV_ERROR(dev, "Failed to enable regulator (%d)\n", ret);return ret;}- }
- if (cond && mipi_dbi_display_is_on(mipi))
return 1;- mipi_dbi_hw_reset(mipi);
- ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET);
- if (ret) {
DRM_DEV_ERROR(dev, "Failed to send reset command (%d)\n", ret);if (mipi->regulator)regulator_disable(mipi->regulator);return ret;- }
- /*
* If we did a hw reset, we know the controller is in Sleep mode and* per MIPI DSC spec should wait 5ms after soft reset. If we didn't,* we assume worst case and wait 120ms.*/- if (mipi->reset)
usleep_range(5000, 20000);- else
msleep(120);- return 0;
+}
+/**
- mipi_dbi_poweron_reset - MIPI DBI poweron and reset
- @mipi: MIPI DBI structure
- This function enables the regulator if used and does a hardware and software
- reset.
- Returns:
- Zero on success, or a negative error code.
- */
+int mipi_dbi_poweron_reset(struct mipi_dbi *mipi) +{
- return mipi_dbi_poweron_reset_conditional(mipi, false);
+} +EXPORT_SYMBOL(mipi_dbi_poweron_reset);
+/**
- mipi_dbi_poweron_conditional_reset - MIPI DBI poweron and conditional reset
- @mipi: MIPI DBI structure
- This function enables the regulator if used and if the display is off, it
- does a hardware and software reset. If mipi_dbi_display_is_on() determines
- that the display is on, no reset is performed.
- Returns:
- Zero if the controller was reset, 1 if the display was already on, or a
- negative error code.
- */
+int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi) +{
- return mipi_dbi_poweron_reset_conditional(mipi, true);
+} +EXPORT_SYMBOL(mipi_dbi_poweron_conditional_reset);
#if IS_ENABLED(CONFIG_SPI)
/**
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 9fd4423c8e70..a6396ef9cc4a 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -179,19 +179,16 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
struct device *dev = tdev->drm->dev; int ret; u8 addr_mode;
DRM_DEBUG_KMS("\n");
mipi_dbi_hw_reset(mipi);
ret = mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f);
if (ret) {
DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
- ret = mipi_dbi_poweron_reset(mipi);
- if (ret) return;
- }
mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f); mipi_dbi_command(mipi, ST7586_OTP_RW_CTRL, 0x00);
msleep(10);
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index 1f38e15da676..650257ad0193 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c
Also need:
@@ -40,7 +40,6 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe, { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); - struct device *dev = tdev->drm->dev; int ret; u8 addr_mode;
to prevent compiler warning.
@@ -46,13 +46,9 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
DRM_DEBUG_KMS("\n");
- mipi_dbi_hw_reset(mipi);
- ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET);
- if (ret) {
DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
- ret = mipi_dbi_poweron_reset(mipi);
- if (ret) return;
}
msleep(150);
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 6441d9a9161a..795a4a2205bb 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -73,6 +73,8 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe); void mipi_dbi_hw_reset(struct mipi_dbi *mipi); bool mipi_dbi_display_is_on(struct mipi_dbi *mipi); +int mipi_dbi_poweron_reset(struct mipi_dbi *mipi); +int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi); u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len);
int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
With the compiler warning fixed:
Reviewed-by: David Lechner david@lechnology.com
Also, the whole series:
Tested-by: David Lechner david@lechnology.com
using st7735r which has a hardware reset and backlight, but no regulator. And it is write-only, so cannot detect that the display is already on.
Den 11.01.2018 23.22, skrev David Lechner:
On 01/10/2018 12:59 PM, Noralf Trønnes wrote:
Split out common poweron-reset functionality.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/tinydrm/mi0283qt.c | 22 ++---------- drivers/gpu/drm/tinydrm/mipi-dbi.c | 73 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/tinydrm/st7586.c | 9 ++--- drivers/gpu/drm/tinydrm/st7735r.c | 8 ++--- include/drm/tinydrm/mipi-dbi.h | 2 ++ 5 files changed, 83 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index c69a4d958f24..1617405faed4 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -49,33 +49,17 @@ static int mi0283qt_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) { - DRM_DEV_ERROR(dev, "Failed to enable regulator %d\n", ret); + ret = mipi_dbi_poweron_conditional_reset(mipi); + if (ret < 0) return ret; - }
- /* Avoid flicker by skipping setup if the bootloader has done it */
It could be helpful to keep this comment.
It's not valid anymore since we're not doing this at probe time.
- if (mipi_dbi_display_is_on(mipi)) + if (ret == 1) return 0; - mipi_dbi_hw_reset(mipi); - ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); - if (ret) { - DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); - regulator_disable(mipi->regulator); - return ret; - }
- msleep(20);
mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF); mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30); diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 1c8ef0c4d6d4..3e879d605ed3 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -463,6 +463,7 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi) val &= ~DCS_POWER_MODE_RESERVED_MASK; + /* The poweron/reset value is 08h DCS_POWER_MODE_DISPLAY_NORMAL_MODE */ if (val != (DCS_POWER_MODE_DISPLAY | DCS_POWER_MODE_DISPLAY_NORMAL_MODE | DCS_POWER_MODE_SLEEP_MODE)) return false; @@ -473,6 +474,78 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi) } EXPORT_SYMBOL(mipi_dbi_display_is_on); +static int mipi_dbi_poweron_reset_conditional(struct mipi_dbi *mipi, bool cond) +{ + struct device *dev = mipi->tinydrm.drm->dev; + int ret;
+ if (mipi->regulator) { + ret = regulator_enable(mipi->regulator); + if (ret) { + DRM_DEV_ERROR(dev, "Failed to enable regulator (%d)\n", ret); + return ret; + } + }
+ if (cond && mipi_dbi_display_is_on(mipi)) + return 1;
+ mipi_dbi_hw_reset(mipi); + ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); + if (ret) { + DRM_DEV_ERROR(dev, "Failed to send reset command (%d)\n", ret); + if (mipi->regulator) + regulator_disable(mipi->regulator); + return ret; + }
+ /* + * If we did a hw reset, we know the controller is in Sleep mode and + * per MIPI DSC spec should wait 5ms after soft reset. If we didn't, + * we assume worst case and wait 120ms. + */ + if (mipi->reset) + usleep_range(5000, 20000); + else + msleep(120);
+ return 0; +}
+/**
- mipi_dbi_poweron_reset - MIPI DBI poweron and reset
- @mipi: MIPI DBI structure
- This function enables the regulator if used and does a hardware
and software
- reset.
- Returns:
- Zero on success, or a negative error code.
- */
+int mipi_dbi_poweron_reset(struct mipi_dbi *mipi) +{ + return mipi_dbi_poweron_reset_conditional(mipi, false); +} +EXPORT_SYMBOL(mipi_dbi_poweron_reset);
+/**
- mipi_dbi_poweron_conditional_reset - MIPI DBI poweron and
conditional reset
- @mipi: MIPI DBI structure
- This function enables the regulator if used and if the display is
off, it
- does a hardware and software reset. If mipi_dbi_display_is_on()
determines
- that the display is on, no reset is performed.
- Returns:
- Zero if the controller was reset, 1 if the display was already
on, or a
- negative error code.
- */
+int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi) +{ + return mipi_dbi_poweron_reset_conditional(mipi, true); +} +EXPORT_SYMBOL(mipi_dbi_poweron_conditional_reset);
#if IS_ENABLED(CONFIG_SPI) /** diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 9fd4423c8e70..a6396ef9cc4a 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -179,19 +179,16 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); - struct device *dev = tdev->drm->dev; int ret; u8 addr_mode; DRM_DEBUG_KMS("\n"); - mipi_dbi_hw_reset(mipi); - ret = mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f); - if (ret) { - DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); + ret = mipi_dbi_poweron_reset(mipi); + if (ret) return; - } + mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f); mipi_dbi_command(mipi, ST7586_OTP_RW_CTRL, 0x00); msleep(10); diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index 1f38e15da676..650257ad0193 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c
Also need:
@@ -40,7 +40,6 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe, { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); - struct device *dev = tdev->drm->dev; int ret; u8 addr_mode;
to prevent compiler warning.
Oops, hadn't enabled the driver...
@@ -46,13 +46,9 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe, DRM_DEBUG_KMS("\n"); - mipi_dbi_hw_reset(mipi);
- ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); - if (ret) { - DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); + ret = mipi_dbi_poweron_reset(mipi); + if (ret) return; - } msleep(150); diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 6441d9a9161a..795a4a2205bb 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -73,6 +73,8 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe); void mipi_dbi_hw_reset(struct mipi_dbi *mipi); bool mipi_dbi_display_is_on(struct mipi_dbi *mipi); +int mipi_dbi_poweron_reset(struct mipi_dbi *mipi); +int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi); u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len); int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
With the compiler warning fixed:
Reviewed-by: David Lechner david@lechnology.com
Also, the whole series:
Tested-by: David Lechner david@lechnology.com
using st7735r which has a hardware reset and backlight, but no regulator. And it is write-only, so cannot detect that the display is already on.
Thanks,
Noralf.
It's better to leave power handling and controller init to the modesetting machinery using the simple pipe .enable and .disable callbacks. Remove unused mipi_dbi_pipe_enable().
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: David Lechner david@lechnology.com --- drivers/gpu/drm/tinydrm/mi0283qt.c | 47 ++++++++------------------------------ drivers/gpu/drm/tinydrm/mipi-dbi.c | 32 ++++---------------------- include/drm/tinydrm/mipi-dbi.h | 2 -- 3 files changed, 15 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 1617405faed4..79cb5af5abac 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -47,8 +47,11 @@ #define ILI9341_MADCTL_MX BIT(6) #define ILI9341_MADCTL_MY BIT(7)
-static int mi0283qt_init(struct mipi_dbi *mipi) +static void mi0283qt_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); u8 addr_mode; int ret;
@@ -56,9 +59,9 @@ static int mi0283qt_init(struct mipi_dbi *mipi)
ret = mipi_dbi_poweron_conditional_reset(mipi); if (ret < 0) - return ret; + return; if (ret == 1) - return 0; + goto out_enable;
mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
@@ -121,19 +124,12 @@ static int mi0283qt_init(struct mipi_dbi *mipi) mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON); msleep(100);
- return 0; -} - -static void mi0283qt_fini(void *data) -{ - struct mipi_dbi *mipi = data; - - DRM_DEBUG_KMS("\n"); - regulator_disable(mipi->regulator); +out_enable: + mipi_dbi_enable_flush(mipi); }
static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = { - .enable = mipi_dbi_pipe_enable, + .enable = mi0283qt_enable, .disable = mipi_dbi_pipe_disable, .update = tinydrm_display_pipe_update, .prepare_fb = tinydrm_display_pipe_prepare_fb, @@ -214,17 +210,6 @@ static int mi0283qt_probe(struct spi_device *spi) if (ret) return ret;
- ret = mi0283qt_init(mipi); - if (ret) - return ret; - - /* use devres to fini after drm unregister (drv->remove is before) */ - ret = devm_add_action(dev, mi0283qt_fini, mipi); - if (ret) { - mi0283qt_fini(mipi); - return ret; - } - spi_set_drvdata(spi, mipi);
return devm_tinydrm_register(&mipi->tinydrm); @@ -240,25 +225,13 @@ static void mi0283qt_shutdown(struct spi_device *spi) static int __maybe_unused mi0283qt_pm_suspend(struct device *dev) { struct mipi_dbi *mipi = dev_get_drvdata(dev); - int ret;
- ret = drm_mode_config_helper_suspend(mipi->tinydrm.drm); - if (ret) - return ret; - - mi0283qt_fini(mipi); - - return 0; + return drm_mode_config_helper_suspend(mipi->tinydrm.drm); }
static int __maybe_unused mi0283qt_pm_resume(struct device *dev) { struct mipi_dbi *mipi = dev_get_drvdata(dev); - int ret; - - ret = mi0283qt_init(mipi); - if (ret) - return ret;
drm_mode_config_helper_resume(mipi->tinydrm.drm);
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 3e879d605ed3..6798b4837441 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -290,31 +290,6 @@ void mipi_dbi_enable_flush(struct mipi_dbi *mipi) } EXPORT_SYMBOL(mipi_dbi_enable_flush);
-/** - * mipi_dbi_pipe_enable - MIPI DBI pipe enable helper - * @pipe: Display pipe - * @crtc_state: CRTC state - * - * This function enables backlight. Drivers can use this as their - * &drm_simple_display_pipe_funcs->enable callback. - */ -void 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"); - - mipi->enabled = true; - if (fb) - fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0); - - tinydrm_enable_backlight(mipi->backlight); -} -EXPORT_SYMBOL(mipi_dbi_pipe_enable); - static void mipi_dbi_blank(struct mipi_dbi *mipi) { struct drm_device *drm = mipi->tinydrm.drm; @@ -336,8 +311,8 @@ static void mipi_dbi_blank(struct mipi_dbi *mipi) * mipi_dbi_pipe_disable - MIPI DBI pipe disable helper * @pipe: Display pipe * - * This function disables backlight if present or if not the - * display memory is blanked. Drivers can use this as their + * This function disables backlight if present, if not the display memory is + * blanked. The regulator is disabled if in use. Drivers can use this as their * &drm_simple_display_pipe_funcs->disable callback. */ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe) @@ -353,6 +328,9 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe) tinydrm_disable_backlight(mipi->backlight); else mipi_dbi_blank(mipi); + + if (mipi->regulator) + regulator_disable(mipi->regulator); } EXPORT_SYMBOL(mipi_dbi_pipe_disable);
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 795a4a2205bb..44e824af2ef6 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -68,8 +68,6 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, struct drm_driver *driver, const struct drm_display_mode *mode, unsigned int rotation); void mipi_dbi_enable_flush(struct mipi_dbi *mipi); -void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state); void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe); void mipi_dbi_hw_reset(struct mipi_dbi *mipi); bool mipi_dbi_display_is_on(struct mipi_dbi *mipi);
Embed the mode in tinydrm_connector instead of doing an devm_ allocation. Remove unnecessary use of ret variable at the end of tinydrm_display_pipe_init().
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: David Lechner david@lechnology.com --- drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 34 +++++++++++------------------ 1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index f41fc506ff87..11ae950b0fc9 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -15,7 +15,7 @@
struct tinydrm_connector { struct drm_connector base; - const struct drm_display_mode *mode; + struct drm_display_mode mode; };
static inline struct tinydrm_connector * @@ -29,7 +29,7 @@ static int tinydrm_connector_get_modes(struct drm_connector *connector) struct tinydrm_connector *tconn = to_tinydrm_connector(connector); struct drm_display_mode *mode;
- mode = drm_mode_duplicate(connector->dev, tconn->mode); + mode = drm_mode_duplicate(connector->dev, &tconn->mode); if (!mode) { DRM_ERROR("Failed to duplicate mode\n"); return 0; @@ -92,7 +92,7 @@ tinydrm_connector_create(struct drm_device *drm, if (!tconn) return ERR_PTR(-ENOMEM);
- tconn->mode = mode; + drm_mode_copy(&tconn->mode, mode); connector = &tconn->base;
drm_connector_helper_add(connector, &tinydrm_connector_hfuncs); @@ -199,35 +199,27 @@ tinydrm_display_pipe_init(struct tinydrm_device *tdev, unsigned int rotation) { struct drm_device *drm = tdev->drm; - struct drm_display_mode *mode_copy; + struct drm_display_mode mode_copy; struct drm_connector *connector; int ret;
- mode_copy = devm_kmalloc(drm->dev, sizeof(*mode_copy), GFP_KERNEL); - if (!mode_copy) - return -ENOMEM; - - *mode_copy = *mode; - ret = tinydrm_rotate_mode(mode_copy, rotation); + drm_mode_copy(&mode_copy, mode); + ret = tinydrm_rotate_mode(&mode_copy, rotation); if (ret) { DRM_ERROR("Illegal rotation value %u\n", rotation); return -EINVAL; }
- drm->mode_config.min_width = mode_copy->hdisplay; - drm->mode_config.max_width = mode_copy->hdisplay; - drm->mode_config.min_height = mode_copy->vdisplay; - drm->mode_config.max_height = mode_copy->vdisplay; + drm->mode_config.min_width = mode_copy.hdisplay; + drm->mode_config.max_width = mode_copy.hdisplay; + drm->mode_config.min_height = mode_copy.vdisplay; + drm->mode_config.max_height = mode_copy.vdisplay;
- connector = tinydrm_connector_create(drm, mode_copy, connector_type); + connector = tinydrm_connector_create(drm, &mode_copy, connector_type); if (IS_ERR(connector)) return PTR_ERR(connector);
- ret = drm_simple_display_pipe_init(drm, &tdev->pipe, funcs, formats, - format_count, NULL, connector); - if (ret) - return ret; - - return 0; + return drm_simple_display_pipe_init(drm, &tdev->pipe, funcs, formats, + format_count, NULL, connector); } EXPORT_SYMBOL(tinydrm_display_pipe_init);
The MIPI DBI spec states that reset active/low time should be more than 9us. Change from 20ms to 20us.
Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/mipi-dbi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 6798b4837441..75dd65c57e74 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -414,7 +414,7 @@ void mipi_dbi_hw_reset(struct mipi_dbi *mipi) return;
gpiod_set_value_cansleep(mipi->reset, 0); - msleep(20); + usleep_range(20, 1000); gpiod_set_value_cansleep(mipi->reset, 1); msleep(120); }
On 01/10/2018 12:59 PM, Noralf Trønnes wrote:
The MIPI DBI spec states that reset active/low time should be more than 9us. Change from 20ms to 20us.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/tinydrm/mipi-dbi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 6798b4837441..75dd65c57e74 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -414,7 +414,7 @@ void mipi_dbi_hw_reset(struct mipi_dbi *mipi) return;
gpiod_set_value_cansleep(mipi->reset, 0);
- msleep(20);
- usleep_range(20, 1000); gpiod_set_value_cansleep(mipi->reset, 1); msleep(120); }
Reviewed-by: David Lechner david@lechnology.com Tested-by: David Lechner david@lechnology.com
Den 10.01.2018 19.59, skrev Noralf Trønnes:
Fix a few things that came up during the tinydrm review but wasn't addressed at the time.
Noralf.
Thanks for review and testing, series applied to drm-misc.
Noralf.
Changes since version 2:
- s/mipi_dbi_por_conditional/mipi_dbi_poweron_reset_conditional/
- Add sleep after soft reset to comply with spec.
- Remove reset msleep from mi0283qt following the previous change.
- Use (ret == 1) to test for display is ON, no reset.
- Add patch to change hard reset active time.
Changes since version 1:
- Add mipi_dbi_enable_flush() instead of the pipe enable helper mipi_dbi_pipe_enable() that doesn't really fit anymore.
- Add common poweron-reset functionality.
- Use drm_mode_copy() instead of direct assignment
Noralf Trønnes (7): drm/tinydrm/mi0283qt: Use common include order drm/tinydrm/mi0283qt: Remove ili9341.h drm/tinydrm/mipi-dbi: Add mipi_dbi_enable_flush() drm/tinydrm/mipi-dbi: Add poweron-reset functions drm/tinydrm/mi0283qt: Let the display pipe handle power drm/tinydrm: Embed the mode in tinydrm_connector drm/tinydrm/mipi-dbi: Change reset active time
drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 34 ++++----- drivers/gpu/drm/tinydrm/ili9225.c | 6 +- drivers/gpu/drm/tinydrm/mi0283qt.c | 106 ++++++++++++---------------- drivers/gpu/drm/tinydrm/mipi-dbi.c | 101 ++++++++++++++++++++++---- drivers/gpu/drm/tinydrm/st7586.c | 15 ++-- drivers/gpu/drm/tinydrm/st7735r.c | 10 +-- include/drm/tinydrm/ili9341.h | 54 -------------- include/drm/tinydrm/mipi-dbi.h | 5 +- 8 files changed, 154 insertions(+), 177 deletions(-) delete mode 100644 include/drm/tinydrm/ili9341.h
dri-devel@lists.freedesktop.org
-
David Lechner -
Noralf Trønnes