Den 09.01.2018 02.38, skrev David Lechner:
On 01/07/2018 11:44 AM, 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 | 20 ++---------- drivers/gpu/drm/tinydrm/mipi-dbi.c | 63 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/tinydrm/st7586.c | 9 ++---- drivers/gpu/drm/tinydrm/st7735r.c | 8 ++--- include/drm/tinydrm/mipi-dbi.h | 2 ++ 5 files changed, 73 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index c69a4d958f24..2a78bcd35045 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -49,31 +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 > 0) return 0;
If I am reading the patch right, it looks like there are two
if (ret > 0) return 0;
in a row with nothing in between when this is applied.
- 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); diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index ecc5c81a93ac..eea6803ff223 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,68 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi) } EXPORT_SYMBOL(mipi_dbi_display_is_on); +static int mipi_dbi_por_conditional(struct mipi_dbi *mipi, bool cond)
I know it is long, but it would be nice to spell out poweron_reset here instead of "por".
+{ + 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);
It seems unnecessary to do a soft reset after a hard reset, but I suppose some displays don't have a hard reset and the extra soft reset shouldn't hurt anything.
Both ILI9341 and ST7735R (not ST7586S) lists soft reset as part of the Power Flow Chart, but it's not explicit about it being required or not:
Power on sequence HW reset SW reset State is now Sleep in
I looked in the MIPI DBI spec and there's nothing about requiring both hw _and_ soft reset. But I have seen hard and soft reset in many panel init's, so I think we keep this as the default. It has a 5ms penalty. I could shave that off in mipi_dbi_hw_reset(). It keeps reset low for 20ms, but the spec says it just has to be longer than 9us with noise spikes less than 20ns wide:
- msleep(20); + usleep_range(20, 1000);
Noralf.
+ if (ret) { + DRM_DEV_ERROR(dev, "Failed to send reset command (%d)\n", ret); + if (mipi->regulator) + regulator_disable(mipi->regulator); + return ret; + }
+ 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_por_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_por_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);