* [PATCH v3 1/7] drm/tinydrm/mi0283qt: Use common include order
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
@ 2018-01-10 18:59 ` Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 2/7] drm/tinydrm/mi0283qt: Remove ili9341.h Noralf Trønnes
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-10 18:59 UTC (permalink / raw)
To: david; +Cc: dri-devel
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)
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 2/7] drm/tinydrm/mi0283qt: Remove ili9341.h
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 1/7] drm/tinydrm/mi0283qt: Use common include order Noralf Trønnes
@ 2018-01-10 18:59 ` Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 3/7] drm/tinydrm/mipi-dbi: Add mipi_dbi_enable_flush() Noralf Trønnes
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-10 18:59 UTC (permalink / raw)
To: david; +Cc: dri-devel
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 */
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 3/7] drm/tinydrm/mipi-dbi: Add mipi_dbi_enable_flush()
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 1/7] drm/tinydrm/mi0283qt: Use common include order Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 2/7] drm/tinydrm/mi0283qt: Remove ili9341.h Noralf Trønnes
@ 2018-01-10 18:59 ` Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 4/7] drm/tinydrm/mipi-dbi: Add poweron-reset functions Noralf Trønnes
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-10 18:59 UTC (permalink / raw)
To: david; +Cc: dri-devel
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);
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 4/7] drm/tinydrm/mipi-dbi: Add poweron-reset functions
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
` (2 preceding siblings ...)
2018-01-10 18:59 ` [PATCH v3 3/7] drm/tinydrm/mipi-dbi: Add mipi_dbi_enable_flush() Noralf Trønnes
@ 2018-01-10 18:59 ` Noralf Trønnes
2018-01-11 22:22 ` David Lechner
2018-01-10 18:59 ` [PATCH v3 5/7] drm/tinydrm/mi0283qt: Let the display pipe handle power Noralf Trønnes
` (3 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-10 18:59 UTC (permalink / raw)
To: david; +Cc: dri-devel
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);
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/7] drm/tinydrm/mipi-dbi: Add poweron-reset functions
2018-01-10 18:59 ` [PATCH v3 4/7] drm/tinydrm/mipi-dbi: Add poweron-reset functions Noralf Trønnes
@ 2018-01-11 22:22 ` David Lechner
2018-01-12 15:08 ` Noralf Trønnes
0 siblings, 1 reply; 12+ messages in thread
From: David Lechner @ 2018-01-11 22:22 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: dri-devel
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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/7] drm/tinydrm/mipi-dbi: Add poweron-reset functions
2018-01-11 22:22 ` David Lechner
@ 2018-01-12 15:08 ` Noralf Trønnes
0 siblings, 0 replies; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-12 15:08 UTC (permalink / raw)
To: David Lechner; +Cc: dri-devel
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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 5/7] drm/tinydrm/mi0283qt: Let the display pipe handle power
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
` (3 preceding siblings ...)
2018-01-10 18:59 ` [PATCH v3 4/7] drm/tinydrm/mipi-dbi: Add poweron-reset functions Noralf Trønnes
@ 2018-01-10 18:59 ` Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 6/7] drm/tinydrm: Embed the mode in tinydrm_connector Noralf Trønnes
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-10 18:59 UTC (permalink / raw)
To: david; +Cc: dri-devel
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);
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 6/7] drm/tinydrm: Embed the mode in tinydrm_connector
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
` (4 preceding siblings ...)
2018-01-10 18:59 ` [PATCH v3 5/7] drm/tinydrm/mi0283qt: Let the display pipe handle power Noralf Trønnes
@ 2018-01-10 18:59 ` Noralf Trønnes
2018-01-10 18:59 ` [PATCH v3 7/7] drm/tinydrm/mipi-dbi: Change reset active time Noralf Trønnes
2018-01-15 14:21 ` [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
7 siblings, 0 replies; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-10 18:59 UTC (permalink / raw)
To: david; +Cc: dri-devel
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);
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 7/7] drm/tinydrm/mipi-dbi: Change reset active time
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
` (5 preceding siblings ...)
2018-01-10 18:59 ` [PATCH v3 6/7] drm/tinydrm: Embed the mode in tinydrm_connector Noralf Trønnes
@ 2018-01-10 18:59 ` Noralf Trønnes
2018-01-11 22:24 ` David Lechner
2018-01-15 14:21 ` [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
7 siblings, 1 reply; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-10 18:59 UTC (permalink / raw)
To: david; +Cc: dri-devel
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);
}
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 7/7] drm/tinydrm/mipi-dbi: Change reset active time
2018-01-10 18:59 ` [PATCH v3 7/7] drm/tinydrm/mipi-dbi: Change reset active time Noralf Trønnes
@ 2018-01-11 22:24 ` David Lechner
0 siblings, 0 replies; 12+ messages in thread
From: David Lechner @ 2018-01-11 22:24 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: dri-devel
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>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 0/7] drm/tinydrm: Cleanup
2018-01-10 18:59 [PATCH v3 0/7] drm/tinydrm: Cleanup Noralf Trønnes
` (6 preceding siblings ...)
2018-01-10 18:59 ` [PATCH v3 7/7] drm/tinydrm/mipi-dbi: Change reset active time Noralf Trønnes
@ 2018-01-15 14:21 ` Noralf Trønnes
7 siblings, 0 replies; 12+ messages in thread
From: Noralf Trønnes @ 2018-01-15 14:21 UTC (permalink / raw)
To: david; +Cc: dri-devel
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 mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread