From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 10/23] drm: omapdrm: Use atomic state instead of local device state Date: Mon, 6 Jun 2016 13:37:13 +0300 Message-ID: <57555259.4060902@ti.com> References: <1461702945-14185-1-git-send-email-laurent.pinchart@ideasonboard.com> <5731E0FB.2080907@ti.com> <20160511073756.GM27098@phenom.ffwll.local> <1554411.Be0AQWmIs6@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0796868367==" Return-path: Received: from comal.ext.ti.com (comal.ext.ti.com [198.47.26.152]) by gabe.freedesktop.org (Postfix) with ESMTPS id CBCAD6E3E4 for ; Mon, 6 Jun 2016 10:37:19 +0000 (UTC) In-Reply-To: <1554411.Be0AQWmIs6@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart , Daniel Vetter Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0796868367== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lvl8JtRP0i1c5FcQnBQpDbke7UWMECm6E" --lvl8JtRP0i1c5FcQnBQpDbke7UWMECm6E Content-Type: multipart/mixed; boundary="IK99FF7xvwrBhE5erNvS2ameS20FeitQi" From: Tomi Valkeinen To: Laurent Pinchart , Daniel Vetter Cc: dri-devel@lists.freedesktop.org Message-ID: <57555259.4060902@ti.com> Subject: Re: [PATCH 10/23] drm: omapdrm: Use atomic state instead of local device state References: <1461702945-14185-1-git-send-email-laurent.pinchart@ideasonboard.com> <5731E0FB.2080907@ti.com> <20160511073756.GM27098@phenom.ffwll.local> <1554411.Be0AQWmIs6@avalon> In-Reply-To: <1554411.Be0AQWmIs6@avalon> --IK99FF7xvwrBhE5erNvS2ameS20FeitQi Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/06/16 04:14, Laurent Pinchart wrote: >>> If the DRM core doesn't track whether a CRTC HW is enabled at the >>> moment, maybe omapdrm should? I guess the above works, but that if() >>> makes me a bit uneasy, as it's not really obvious, and the logic behi= nd >>> it could possibly change later... >>> >>> A "if (crtc->is_hw_enabled)" would be much more readable. >=20 > The whole point of this patch is to remove local state and rely on DRM = core=20 > state, so I'd like to avoid that if possible. Yep, but if DRM core doesn't give that information... Using drm_atomic_crtc_needs_modeset() to check if a crtc is enabled at a particular point in the commit sequence feels a bit risky to me. You do explain it in the comment, so it's not that bad, but I'd still rather see an 'if (is-the-hw-enabled-or-not)' than looking at seemingly unrelated information, and deducing from that if the hw is enabled or not= =2E Tomi --IK99FF7xvwrBhE5erNvS2ameS20FeitQi-- --lvl8JtRP0i1c5FcQnBQpDbke7UWMECm6E Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXVVJZAAoJEPo9qoy8lh717bIQAI87Z4f8E9Egy5it4cn8KX8A 0wquux2e4rkgnAvLSPYoeIJvqhIZk5VDkM65GKngWawY0gawYI9LQu8zmPV/w584 42jxdw8X0txrR314Mf/55UbRJZjKmh4BRUL5mjdCB+GuYyVKtSCc3mK7MpvbHPYt dVuhnv90SNolwBS88pTVJlxrGKaBAR0T2HPpylLL+NZCoNPM/QFmEHLGiXhplHin MdbqYWRfJ5S11TcH7ViN729Z18IHVRYAoibV9nG+XmnDorxYxLOaKWPvXOzEeFID Smwmk5Wg+yOxnA6pC4VfZNlBLaQWfCBCvrnr6nKyjBh5bxTsSzNKG604/NsNVPBd Hyv96qNunBlFjTlLe+tMrkBYz+BSDhwy8T8mmTnxITdnMasJG3ykgk2ryfbeaJYE 7ja/2yUk/+pR+eGKWfleaW3RxcBb9h166zmQcy1fbh3zM3+49IAqKZIQ5kWAG4Mb xE7MkfC6oU8VZW3Nh8cVR7Xi4MzOchx2o2UPHAwc1jxxvEG0Rl+xEXi+n5Rxn4pm DSj/gEYPa6+nhl9MrI/DuvtvFjtU/sfDtZ1bfRKlzH8EAvARFB+6P+fwGwetBFls NRvf3+uXs6c0PQRP2tN7dDdc2lYMJA3fN5ZgPAC3yCeXQGKk/NTdA1BJOAcmkLIV ZP1Jp0sOYcd3O8uOWufT =2qdJ -----END PGP SIGNATURE----- --lvl8JtRP0i1c5FcQnBQpDbke7UWMECm6E-- --===============0796868367== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0796868367==--