Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 16 Jul 2020 16:32:09 +0200
From:   =?utf-8?Q?Ond=C5=99ej?= Jirman <megous@megous.com>
To:     Guido =?utf-8?Q?G=C3=BCnther?= <agx@sigxcpu.org>
Cc:     David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
        Thierry Reding <thierry.reding@gmail.com>,
        Sam Ravnborg <sam@ravnborg.org>,
        Fabio Estevam <festevam@gmail.com>,
        Robert Chiras <robert.chiras@nxp.com>,
        Samuel Holland <samuel@sholland.org>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] Fix st7703 panel initialization failures
Message-ID: <20200716143209.ud6ote4q545bo2c7@core.my.home>
Mail-Followup-To: =?utf-8?Q?Ond=C5=99ej?= Jirman <megous@megous.com>,
        Guido =?utf-8?Q?G=C3=BCnther?= <agx@sigxcpu.org>,
        David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
        Thierry Reding <thierry.reding@gmail.com>,
        Sam Ravnborg <sam@ravnborg.org>, Fabio Estevam <festevam@gmail.com>,
        Robert Chiras <robert.chiras@nxp.com>,
        Samuel Holland <samuel@sholland.org>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20200716123753.3552425-1-megous@megous.com>
 <20200716140843.GA359122@bogon.m.sigxcpu.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200716140843.GA359122@bogon.m.sigxcpu.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Guido,

On Thu, Jul 16, 2020 at 04:08:43PM +0200, Guido G?nther wrote:
> Hi Ondrej,
> On Thu, Jul 16, 2020 at 02:37:51PM +0200, Ondrej Jirman wrote:
> > When extending the driver for xbd599 panel support I tried to do minimal
> > changes and keep the existing initialization timing.
> > 
> > It turned out that it's not good enough and the existing init sequence
> > is too aggressive and doesn't follow the specification. On PinePhone
> > panel is being powered down/up during suspend/resume and with current
> > timings this frequently leads to corrupted display.
> 
> Given the amount of ST7703 look alikes i don't think you can go by the
> datasheet and hope not to break other panels. The current sleeps cater
> for the rocktech panel (which suffered from similar issues you describe
> when we took other parameters) so you need to make those panel specific.

It should work on rocktech too. The patch mostly increases/reorders the delays
slightly, to match the controller documentation. I don't see a reason to
complicate the driver with per panel special delays, unless these patches don't
work on your panel.

The init sequence is still suboptimal, and doesn't follow the kernel docs
completely, even after these patches. Controller spec also talks about adding
some delay before enabling the backlight to avoid visual glitches.

Which is what enable callback is documented to be for. Currently part of the
initialization that belongs to prepare callback is also done in enable callback.

I see the glitch (small vertical shift of the image on powerup), but personally
don't care much to introduce even more delays to the driver, just for the
cosmetic issue.

regards,
	o.

> Cheers,
>  -- Guido
> 
> > 
> > This patch series fixes the problems.
> > 
> > The issue was reported by Samuel Holland.
> > 
> > Relevant screenshots from the datasheet:
> > 
> >   Power on timing: https://megous.com/dl/tmp/35b72e674ce0ca27.png
> >   Power off timing: https://megous.com/dl/tmp/dea195517106ff17.png
> >   More optimal reset on poweron: https://megous.com/dl/tmp/a9e5caf14e1b0dc6.png
> >   Less optimal reset on poweron: https://megous.com/dl/tmp/246761039283c4cf.png
> >   Datasheet: https://megous.com/dl/tmp/ST7703_DS_v01_20160128.pdf
> > 
> > Please take a look.
> > 
> > thank you and regards,
> >   Ondrej Jirman
> > 
> > Ondrej Jirman (2):
> >   drm/panel: st7703: Make the sleep exit timing match the spec
> >   drm/panel: st7703: Fix the power up sequence of the panel
> > 
> >  drivers/gpu/drm/panel/panel-sitronix-st7703.c | 29 ++++++++++---------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> > 
> > -- 
> > 2.27.0
> >