Message-ID: <CAMty3ZDTwAx6aY=KJCWjwRo-JxBKENTfPtjV_LkY=QaSxAjRWQ@mail.gmail.com>
Date:   Sat, 12 Jan 2019 22:02:05 +0530
From:   Jagan Teki <jagan@...rulasolutions.com>
To:     Sam Ravnborg <sam@...nborg.org>
Cc:     David Airlie <airlied@...ux.ie>, Sean Paul <sean@...rly.run>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Michael Trimarchi <michael@...rulasolutions.com>,
        linux-amarula@...rulasolutions.com
Subject: Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

Hi Sam,

On Sat, Jan 12, 2019 at 9:03 PM Sam Ravnborg <sam@...nborg.org> wrote:
>
> Hi Jagan.
>
> > > But as we just assert reset (set it to 0), this timing constraint can be ignored.
> >
> > But we unaware of reset pulse duration right? it's the hardware that
> > bring the reset assert if we set the line 0. am I correct or do we
> > need to explicitly wait 10us after reset initiated?
> > there is family of chip Sitronix st7789v which don't taking care of
> > this sequence (I don't know why?) in
> > drivers/gpu/drm/panel/panel-sitronix-st7789v.c with Page 48 of
> > datasheet[2]
>
>
> The prepare sequence used is:
> 1) reset
>    20 ms
> 2) regulator enable
>    20 ms
> 3) unassert reset
>    20 ms
> 4) assert reset
>    30 ms
> 4) unassert reset
>    150 ms
> 5) SOFT RESET
>    5 ms
> 6) Exit SLEEP mode
>    sleep_delay (120 ms)
> 7) COMMANDS
>
> The enable sequence is
> 1) Display ON
> 2) backlight enable
>
>
> The disable sequence is
> 1) backlight disable
> 2) Display OFF
>
> The unprepare sequence is
> 1) Exit SLEEP mode  <= this should be Enter SLEEP mode (covered by previous mail)
>    sleep_delay (120 ms)
> 2) assert reset
> 3) unassert reset
> 4) assert reset
> 5) regulator disable
>
>
> The implementation in simple-panel supports a long list of panels so we
> can assume this is a good reference implementation.
> Unless your combo of st7701 + panel has special requirements.
>
> Prepare sequence:
> -----------------
> - Just set reset to 1 - no need to trigger an edge.
>   But if you somehow think the edge is required keep the current code
>   but drop the delays that are not required.
>   We can assume power is OK when it is enabled, no extra waiting required.
> - No reason to unassert and then assert and then unassert reset.
>
> In other words:
> - Delay between 1 and 2 can be dropped.
> - step 3 and step 4 can be dropped
>
> Unprepare sequence:
> -------------------
> Enter SLEEP mode may take 120 ms so delay between 1) and 2) is OK.
> No reason to do the reset dance, drop 2) and 3)
> Make sure reset is completed, so wait before leaving the function.
> See note 3. on page 54 in datasheet:
>
>         "During the Resetting period, the display will be blanked
>          (The display is entering blanking sequence, which maximum
>          time is 120 ms, when Reset Starts in Sleep Out –mode.
>          The display remains the blank state in Sleep In –mode.) and
>           then return to Default condition for Hardware Reset."
>
> We have already sent an Enter SLEEP command but we have no timing constrains
> when panel is in SLEEP mode, So stick to the 120 ms.
> Just use sleep_delay.

So we need this delay even after reset unlike SLEEP mode delay. OK,
thanks for the points will try to check all these points.

When did .unprepare and .disable are actually called? I turn-off the
backlight by echo 1 > /sys/class/backlight/backlight/bl_power and even
power of the board I assume the video transmission stop so it would
ended-up calling these, but I couldn't see prints. does it the same
time uart dead or something? do you have any inputs on sanity testing
of typical panels, if yes please share.

Jagan.

Powered by blists - more mailing lists