From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761636AbcIZGbo (ORCPT ); Mon, 26 Sep 2016 02:31:44 -0400 Received: from down.free-electrons.com ([37.187.137.238]:38652 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1758880AbcIZGbl (ORCPT ); Mon, 26 Sep 2016 02:31:41 -0400 Date: Sat, 24 Sep 2016 22:18:13 +0200 From: Maxime Ripard To: Jonathan Liu Cc: David Airlie , Chen-Yu Tsai , "dri-devel@lists.freedesktop.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] drm/sun4i: rgb: Enable panel after controller Message-ID: <20160924201813.GE16901@lukather> References: <20160921130304.3486-1-net147@gmail.com> <20160921210340.GI8719@lukather> <20160923131626.GX8719@lukather> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ep0oHQY+/Gbo/zt0" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --ep0oHQY+/Gbo/zt0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 23, 2016 at 11:43:55PM +1000, Jonathan Liu wrote: > Hi Maxime, >=20 > On 23 September 2016 at 23:16, Maxime Ripard > wrote: > > On Thu, Sep 22, 2016 at 08:03:31AM +1000, Jonathan Liu wrote: > >> Hi Maxime, > >> > >> On Thursday, 22 September 2016, Maxime Ripard >> com> wrote: > >> > >> > On Wed, Sep 21, 2016 at 11:03:04PM +1000, Jonathan Liu wrote: > >> > > The panel should be enabled after the controller so that the panel > >> > > prepare/enable delays are properly taken into account. Similarly, = the > >> > > panel should be disabled before the controller so that the panel > >> > > unprepare/disable delays are properly taken into account. > >> > > > >> > > This is useful for avoiding visual glitches. > >> > > >> > This is not really taking any delays into account, especially since > >> > drm_panel_enable and prepare are suppose to block until their > >> > operation is complete. > >> > >> > >> drm_panel_prepare turns on power to the LCD using enable-gpios propert= y of > >> the panel and then blocks for prepare delay. The prepare delay for pan= el > >> can be set to how long it takes between the time the panel is powered = to > >> when it is ready to receive images. If backlight property is specified= the > >> backlight will be off while the panel is powered on. > >> > >> drm_panel_enable blocks for enable delay and then turns on the backlig= ht. > >> The enable delay can be set to how long it takes for panel to start ma= king > >> the image visible after receiving the first valid frame. For example i= f the > >> panel starts off as white and the TFT takes some time to initialize to > >> black before it shows the image being received. > >> > >> Refer to drivers/gpu/drm/panel-panel.simple.c for details. > > > > From drm_panel.h: > > > > """ > > * drm_panel_enable - enable a panel > > * @panel: DRM panel > > * > > * Calling this function will cause the panel display drivers to be turn= ed on > > * and the backlight to be enabled. Content will be visible on screen af= ter > > * this call completes. > > """ > > > > """ > > * drm_panel_prepare - power on a panel > > * @panel: DRM panel > > * > > * Calling this function will enable power and deassert any reset signal= s to > > * the panel. After this has completed it is possible to communicate wit= h any > > * integrated circuitry via a command bus. > > """ > > > > Those comments clearly says that the caller should not have to deal > > with the delays, even more so by just moving calls around and hoping > > that the code running in between is adding enough delay for the panel > > to behave properly. > > > > Maxime > > > > -- > > Maxime Ripard, Free Electrons > > Embedded Linux and Kernel engineering > > http://free-electrons.com >=20 > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/incl= ude/drm/drm_panel.h#n34 >=20 > In comment for struct drm_panel_funcs: > /** > * struct drm_panel_funcs - perform operations on a given panel > * @disable: disable panel (turn off back light, etc.) > * @unprepare: turn off panel > * @prepare: turn on panel and perform set up > * @enable: enable panel (turn on back light, etc.) > * @get_modes: add modes to the connector that the panel is attached to a= nd > * return the number of modes added > * @get_timings: copy display timings into the provided array and return > * the number of display timings available > * > * The .prepare() function is typically called before the display control= ler > * starts to transmit video data. Panel drivers can use this to turn the = panel > * on and wait for it to become ready. If additional configuration is req= uired > * (via a control bus such as I2C, SPI or DSI for example) this is a good= time > * to do that. > * > * After the display controller has started transmitting video data, it's= safe > * to call the .enable() function. This will typically enable the backlig= ht to > * make the image on screen visible. Some panels require a certain amount= of > * time or frames before the image is displayed. This function is respons= ible > * for taking this into account before enabling the backlight to avoid vi= sual > * glitches. > * > * Before stopping video transmission from the display controller it can = be > * necessary to turn off the panel to avoid visual glitches. This is done= in > * the .disable() function. Analogously to .enable() this typically invol= ves > * turning off the backlight and waiting for some time to make sure no im= age > * is visible on the panel. It is then safe for the display controller to > * cease transmission of video data. > * > * To save power when no video data is transmitted, a driver can power do= wn > * the panel. This is the job of the .unprepare() function. > */ It kind of make my point. When drm_panel_enable is called, the content is visible, and there's no extra delay needed. If what you want to fix is that the panel is enabled before the controller, hence resulting in garbage on the screen while the controller is setup, then your commit log is seriously misleading. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --ep0oHQY+/Gbo/zt0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJX5t+FAAoJEBx+YmzsjxAgPuUP/RzTAVxQYgn4OZCSSLwsc64Z 7/Bx2MucLIx0GD8I06DdsXmq200fdRQbZ6rgnUIzMII7wTMQYDnB1w26+i2ORAku u28fDKv21HuWpdbmRzcVq1CFJCc8lDugs55eLxsqXhWqE6PjqYIdzDBZN7STLzEq G5qf1N+KERbNgDfKDLE6rktAhdebpSRhz+dk5MkSJnQZSlP/udE68Gd+UHJigAlS GkMXEigFS89t9P+XKuGGdIvH+azJHs13qGj/4VxUpo6JRS+bG7nMt8GkKBXtqdHK XW7gj5fZhnyualSTOvXhCiQtOFSlXMs9Akkp81Pq3rYFCKv/gZu78bC5BgZZZQ2P 4rKOtiteBSKvXomXWsSHlGOp7QhWQTOu8zW+BV2BwNIbebHo9Z2unc0j6xZTMuUi DsSguNoAi3/FW7IpPgEYhWiAdw4SYKE5BK0CWI/PIvxPGp2MMapX66dubc2XmdaI e/OCBdhTM0btWQj/FPR4BsObUy4d7C8w+WFhNPUxi4V1o6JOjf5pM7ag1B2Uv2X1 mnkm3INgNxK2zpn8EcfXWOzYnNESOgY3h4Hn5hy2k3GG2B5IMCcUTgpOqCID539M 1s1ahd2BsCCb2UWnH2A5xCG2ssL/v8stUavNokvdUacvHUPyZUtS4/zHKqGB+tXt 3A8bscRl+OhndCT3UIeM =vdWj -----END PGP SIGNATURE----- --ep0oHQY+/Gbo/zt0--