[PATCH v2 4/6] drm/tinydrm/mipi-dbi: Add poweron-reset functions
Noralf Trønnes
noralf at tronnes.org
Tue Jan 9 15:01:33 UTC 2018
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 at 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);
>>
>
More information about the dri-devel
mailing list