From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 8/9] drm/atomic: Remove drm_atomic_connectors_for_crtc. Date: Mon, 7 Dec 2015 11:34:44 +0100 Message-ID: <20151207103444.GB13177@ulmo> References: <1448357676-27837-1-git-send-email-maarten.lankhorst@linux.intel.com> <1448357676-27837-9-git-send-email-maarten.lankhorst@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0431758942==" Return-path: In-Reply-To: <1448357676-27837-9-git-send-email-maarten.lankhorst@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Maarten Lankhorst Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0431758942== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="PmA2V3Z32TCmWXqI" Content-Disposition: inline --PmA2V3Z32TCmWXqI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 24, 2015 at 10:34:35AM +0100, Maarten Lankhorst wrote: [...] > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_at= omic_helper.c > index cee31d43cd1c..fb79c54b2aed 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -420,8 +420,6 @@ drm_atomic_helper_check_modeset(struct drm_device *de= v, > * crtc only changed its mode but has the same set of connectors. > */ > for_each_crtc_in_state(state, crtc, crtc_state, i) { > - int num_connectors; > - > /* > * We must set ->active_changed after walking connectors for > * otherwise an update that only changes active would result in > @@ -449,10 +447,7 @@ drm_atomic_helper_check_modeset(struct drm_device *d= ev, > if (ret !=3D 0) > return ret; > =20 > - num_connectors =3D drm_atomic_connectors_for_crtc(state, > - crtc); > - > - if (crtc_state->enable !=3D !!num_connectors) { > + if (crtc_state->enable !=3D !!crtc_state->connector_mask) { I have difficulty to doubly negate in my head, so something like this would be a lot clearer in my opinion: bool enable =3D crtc_state->connector_mask !=3D 0; ... if (crtc_state->enable !=3D enable) ... Or perhaps even: bool disable =3D crtc_state->connector_mask =3D=3D 0; ... if (crtc_state->enable =3D=3D disable) ... > DRM_DEBUG_ATOMIC("[CRTC:%d] enabled/connectors mismatch\n", > crtc->base.id); > =20 > @@ -1668,7 +1663,7 @@ static int update_output_state(struct drm_atomic_st= ate *state, > if (crtc =3D=3D set->crtc) > continue; > =20 > - if (!drm_atomic_connectors_for_crtc(state, crtc)) { > + if (!crtc_state->connector_mask) { Similarly I think it would be more natural to say: if (crtc->state->connector_mask =3D=3D 0) here. Anyway, this is mostly about personal taste, and the change looks correct to me (after checking twice that I got the double negation right), so: Reviewed-by: Thierry Reding --PmA2V3Z32TCmWXqI Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWZWDCAAoJEN0jrNd/PrOhhD0QAIF9jgXo2nBNGMuE6cu01N8H EKLNtigffmCQN5XPvZ70d5lOqUekRgQchqPZtaJXQjaP/xOobWRTBkLulV7NXCEI VVix4EsZ/4hfUjTj7bDqjxKM29aDRrX6QWfeEC4U4938XHJSMNwnDIzbGrsTTdel 4oiJoaIvtN/D2LlwmhcL2Xag7BCmzaiqCMwiK8p4K9ghXXUAb2IqtSl61nhpbuNp m+cFhP+ZHonNhTcXYUMOzd0N2dup0pSe3NsVZzGkykK147k4RJZK/+o2ThbLhBSj Zus6T5Jn6apmHFydP2iQOcVeXizkgq/eOo7ZnKtNSvNZe0/TEA3jvc7t8wObxVPS S9XZH6bc8xVJ9xyTg0/isu5ceO1HfmclYSncx6lcV0Yf+zFMWGX2VUdRcBsdztfM InMpnJsYjJbLyTDJBxnA8f6zJBh3FmvCy246WAwTZtlZ6PqZmw4YDfQkgCtnRLCl VeUKdW3cAOifFlpLvpM1z01JhP/+swuHnWCaCjW52qJBcIFuw/XgXS+rNQG8kz5s C5nkqqSa+3/EY+RW9QcSoDOSG4rK7mwSKGCJxs2WIqG+/hgV6aSxV4/nssTr4pyT PssFffSu+cHDacflnszjP7XRgrcIyCnDIdEONQbdTR/VOjzqMmwhI/9RQlew2OVI VumCrudiE37Kix54C/4X =0A6A -----END PGP SIGNATURE----- --PmA2V3Z32TCmWXqI-- --===============0431758942== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============0431758942==--