From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 02/17] drm/tegra: Do not enable output on .mode_set() Date: Tue, 4 Nov 2014 16:26:12 +0100 Message-ID: <20141104152610.GF31200@ulmo> References: <1415006868-318-1-git-send-email-thierry.reding@gmail.com> <1415006868-318-2-git-send-email-thierry.reding@gmail.com> <20141104145012.GE31200@ulmo> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0906586601==" Return-path: Received: from mail-wg0-f53.google.com (mail-wg0-f53.google.com [74.125.82.53]) by gabe.freedesktop.org (Postfix) with ESMTP id 483DE6E09B for ; Tue, 4 Nov 2014 07:26:15 -0800 (PST) Received: by mail-wg0-f53.google.com with SMTP id b13so13826526wgh.12 for ; Tue, 04 Nov 2014 07:26:14 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sean Paul Cc: dri-devel List-Id: dri-devel@lists.freedesktop.org --===============0906586601== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="s5/bjXLgkIwAv6Hi" Content-Disposition: inline --s5/bjXLgkIwAv6Hi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 04, 2014 at 10:11:22AM -0500, Sean Paul wrote: > On Tue, Nov 4, 2014 at 9:50 AM, Thierry Reding = wrote: > > On Mon, Nov 03, 2014 at 12:18:30PM -0500, Sean Paul wrote: > >> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding wrote: > >> > From: Thierry Reding > >> > > >> > The output is already enabled in .dpms(), doing it in .mode_set() too > >> > can cause noticeable flicker. > >> > > >> > >> I think this should be coupled with "drm/tegra: DPMS off/on in encoder > >> prepare/commit" that I sent earlier this week. Without it, the driver > >> can get into a state where connector status is on, but the output is > >> disabled. > > > > I'm not sure I exactly understand which problem that patch fixes, but > > I'll give it some testing to see if it doesn't break anything. > > >=20 > I'll try to explain it more clearly. >=20 > The problem occurs when userspace does set_property(dpms_on), then modese= t. >=20 > When the set_property ioctl to set dpms =3D on is called, the tegra > driver goes through drm_helper_connector_dpms(). At this point, > because the connector has not participated in a modeset, > connector->encoder =3D=3D NULL. In drm_helper_connector_dpms(), we set > connector->dpms =3D DRM_MODE_DPMS_ON, and skip everything else because > !encoder && !crtc. >=20 > When modeset is called, we go through drm_crtc_helper_set_config(). > This function will populate connector->encoder with the appropriate > encoder and call drm_crtc_helper_set_mode(), which goes through the > prepare/mode_set/commit calls. Finally, drm_crtc_helper_set_config() > iterates through the connectors involved in the modeset and calls > their dpms() func. In our case, as above, we go through > drm_helper_connector_dpms(), but it just exits early because mode =3D=3D > connector->dpms. >=20 > So we end up in a state where connector->dpms =3D=3D DRM_MODE_DPMS_ON, and > the output, as well as any connected panel, has never been enabled. > The reason is that they're only enabled in encoder_funcs->dpms(), > which is never called in the above scenario. >=20 > I hope that makes more sense :-) Yes, perfect sense. It's somewhat unfortunate that the helpers even allow this to happen. It doesn't seem like it's only the Tegra driver using it wrongly. Anyway, I've applied this to my tree. Thanks, Thierry --s5/bjXLgkIwAv6Hi Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUWPASAAoJEN0jrNd/PrOh/XwP/i9kfjuj+pquIC3WvpKwhyjX nGfZzoAO5VnzKkibfqwnxMzPJUw8E36YDregLvGcmzbVLjmHKlKCk0wQk9lStvm1 +V8DUObNrvMbn0thSLZ+wPYCGRNVH2cSUQe1PC/cruf8XqVRnhvSiPdaVAVPJvqw xpyx7p4u/U/mwBRLQIe3wweita+GnNiWjt/rGLVeqICRRpYtnpdgLJtdmGh5eCKl FoSrRyNrgL79TVUXYMd97FjfQ9UWvEuSlLFexgEIZlMVv7TmBo6yWmswUa9sxZ4C lI4CnIs0RNd+lmeUQWXF61/GmgQiclj5Qr+p1XHHWfwbsKjHei4NYLCzJBnPAruE m5sdbK5XVLJ1FKY8JcOY/appt8C9t5LYRoV2c84atFsfNcQVAFYad1k07CBNhQtT hYt5LI1nXtB9uRwOdj8vISVFvVVgYSWRlQH/PTWb/SGOSTEv08jX0JPJ879c0rZR YoSSb7V9l2L1VbCyuklXVg0YZFsquMW1dCRB1wxrBkkSNdn9XZXg6jJi5LhFY4Uu ZMgDEqC4aXCyhlE8DX+123ZPToPis1VQbLjso1BFC0OfbxlwWYEMlOZUTzGyY4SW qbQegYiNTehtam+VR5twt24mkM7JMM22xuJ9IFv92JTufJYq6v7C5P90uQJoldtI HyIEk11XHsXWipNhKB6p =ljuV -----END PGP SIGNATURE----- --s5/bjXLgkIwAv6Hi-- --===============0906586601== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============0906586601==--