From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v2 3/6] drm/plane: Add optional ->atomic_disable() callback Date: Tue, 25 Nov 2014 12:57:04 +0100 Message-ID: <20141125115702.GB29735@ulmo> References: <1416913789-23209-1-git-send-email-thierry.reding@gmail.com> <1416913789-23209-3-git-send-email-thierry.reding@gmail.com> <20141125112234.GX25711@phenom.ffwll.local> <20141125114544.GA29735@ulmo> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1162109493==" Return-path: Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 331126E74F for ; Tue, 25 Nov 2014 03:57:07 -0800 (PST) Received: by mail-wi0-f177.google.com with SMTP id l15so1155459wiw.10 for ; Tue, 25 Nov 2014 03:57:06 -0800 (PST) In-Reply-To: <20141125114544.GA29735@ulmo> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Daniel Vetter , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1162109493== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="dc+cDN39EJAMEtIO" Content-Disposition: inline --dc+cDN39EJAMEtIO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 25, 2014 at 12:45:46PM +0100, Thierry Reding wrote: > On Tue, Nov 25, 2014 at 12:22:34PM +0100, Daniel Vetter wrote: > > On Tue, Nov 25, 2014 at 12:09:46PM +0100, Thierry Reding wrote: [...] > > > +/* > > > + * drm_plane_disabled - check whether a plane is being disabled > > > + * @plane: plane object > > > + * @old_state: previous atomic state > > > + * > > > + * Checks the atomic state of a plane to determine whether it's bein= g disabled > > > + * or not. This also WARNs if it detects an invalid state (both CRTC= and FB > > > + * need to either both be NULL or both be non-NULL). > > > + * > > > + * RETURNS: > > > + * True if the plane is being disabled, false otherwise. > > > + */ > > > +static inline bool drm_plane_disabled(struct drm_plane *plane, > > > + struct drm_plane_state *old_state) > > > +{ [...] > > > + return (!old_state || old_state->crtc) && !plane->state->crtc; > >=20 > > The !old_state check here confused me a bit, until I've realized that y= ou > > use this for transitional helpers too. What about adding > >=20 > > /* Cope with NULL old_state for transitional helpers. */ > >=20 > > right above? >=20 > Sounds good. When I now thought about the reason again it took me a while to reconstruct, so I figured I'd be extra verbose and used this: /* * When using the transitional helpers, old_state may be NULL. If so, * we know nothing about the current state and have to assume that it * might be enabled. */ Does that sound accurate and sufficient to you? Thierry --dc+cDN39EJAMEtIO Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUdG6OAAoJEN0jrNd/PrOh2bkQALA83l/Jekk2l2ZECrDQlEjm boEmm8hqQxlhtX0AZXTYJtESyiN4x3j07wY4xs3YyDo2zBwomZdDuaoXQnxp4z6p OM5MjcB1uaLpUNWaV2lzAXxg94j1QeBul8hI2cSB/fO6sqM3J8ek0chQhmNLUxPb AVMYCqi6MuLRk+r/zkJUwVWcjkjr9vynPW3vJfKJsr+I11tO/Ntiz6sNiKA64dqa up391k2hgMTFyEsfGamhMW7VXyhrB+foZHpOEEAnTaW8xvJ3C+MEktP60l0C5vOG J6pqEf5gNtvqi7r1kFkzRp/RgFIMX/dLcFMkTz7VOPEybkFZLYJ1RpRmZ3m5K9Mu ygN+aLdowKEHd11pgGn66Cu8j1zp/ZwCrUD7nxi2vqUaVfWE5YA3OqWyHjn2OChW 3xVRvzMwyCMFZtHhjU9DPno6EfJP0EqaeUezzr97bIWL6SEUc6onrLJ2xzD0KVTD 6ClNWoEnhCtl4fWo+j7WeGUDcktC7ZpZiHnrZNWxAJeMyxj4+jZXEi0Dynis+CWX aK+y6od1Q9GJkmHd2hLvgJFGSjHOq9qkAUXJXKcBh7qQ9fVCy2Sd0OWAzTkpPBIa e8Z7PI3vnknRf2pk4dunKJLcBr7Gj0aYLIrmPDrImloChF1rMoseioMGVmLwAJH6 XzSwrFlgmWR32w4Y8Evj =zc0F -----END PGP SIGNATURE----- --dc+cDN39EJAMEtIO-- --===============1162109493== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1162109493==--