Re: [PATCH v3 4/7] drm/tinydrm/mipi-dbi: Add poweron-reset functions

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.