From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [RFC] drm: add plane iterator macros Date: Fri, 21 Nov 2014 17:29:32 +0100 Message-ID: <20141121162930.GA16290@ulmo.nvidia.com> References: <1416573551-1890-1-git-send-email-robdclark@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1981678624==" Return-path: Received: from mail-pa0-f47.google.com (mail-pa0-f47.google.com [209.85.220.47]) by gabe.freedesktop.org (Postfix) with ESMTP id 117FF6E644 for ; Fri, 21 Nov 2014 08:29:38 -0800 (PST) Received: by mail-pa0-f47.google.com with SMTP id kq14so5185396pab.34 for ; Fri, 21 Nov 2014 08:29:37 -0800 (PST) In-Reply-To: <1416573551-1890-1-git-send-email-robdclark@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Clark Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1981678624== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="CE+1k2dSO48ffgeK" Content-Disposition: inline --CE+1k2dSO48ffgeK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 21, 2014 at 07:39:11AM -0500, Rob Clark wrote: > Inspired in part by some cute iterator macros I noticed in i915. I > currently have these in drm/msm, but seems like they should be useful > for others. Quite possibly other iterators would be useful to add for > other drivers. >=20 > Signed-off-by: Rob Clark > --- > NOTE: to actually merge this, depending on the order this is merged > vs drm/msm/mdp5 atomic support, these would have to be removed from > msm_kms.h. >=20 > include/drm/drm_crtc.h | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) >=20 > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index bc1cc3c..5ea46ec 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -773,6 +773,34 @@ struct drm_plane { > struct drm_plane_state *state; > }; > =20 > +/* iterate over the planes *currently* attached to a crtc, useful > + * to apply current state to hw: > + */ > +#define for_each_plane_on_crtc(_crtc, _plane) \ > + list_for_each_entry((_plane), &(_crtc)->dev->mode_config.plane_list, he= ad) \ > + if ((_plane)->state->crtc =3D=3D (_crtc)) Perhaps throw in a drm_ prefix (for this and the below iterator). Or maybe rename to something like: drm_crtc_for_each_plane() to make the argument list more intuitive? > +static inline bool > +__plane_will_be_attached_to_crtc(struct drm_atomic_state *state, > + struct drm_plane *plane, struct drm_crtc *crtc) > +{ > + int idx =3D drm_plane_index(plane); > + > + /* if plane is modified in incoming state, use the new state: */ > + if (state->plane_states[idx]) > + return state->plane_states[idx]->crtc =3D=3D crtc; > + > + /* otherwise, current state: */ > + return plane->state->crtc =3D=3D crtc; > +} Maybe drm_crtc_plane_pending(crtc, state, plane) for somewhat more cohesive naming? > + > +/* iterate over the planes that *will be* attached to a crtc, > + * useful for ->atomic_check() operations: > + */ > +#define for_each_pending_plane_on_crtc(_state, _crtc, _plane) \ > + list_for_each_entry((_plane), &(_crtc)->dev->mode_config.plane_list, he= ad) \ > + if (__plane_will_be_attached_to_crtc((_state), (_plane), (_crtc))) Similarily as for the above, perhaps: drm_crtc_for_each_pending_plane(_crtc, _state, _plane) might be more intuitive. Irrespective of the bike-shedding and with Daniel's comments addressed, this is: Reviewed-by: Thierry Reding --CE+1k2dSO48ffgeK Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUb2hpAAoJEN0jrNd/PrOhn8wP/1QYuwLh7bMLbl6Mdc0e5B+R ybv7pAPdHHBhZeblRtQ2jrX9GRPkXi1QrhQ5nkp0bLSKLXsOF8MMiBOpGZ5OIBM4 eE+zmjK3oRstimGPXXlOP4ntztcbO1dbG/vxuDpSgVaBfqkGUgW41rETPhlPcOGv MNDffZCXR1ziAhyh++DpOkWhhusRmRlalc52szKREVxQvgBRrwDSkv8rGL/EoSZf SiR25Xwzomh4aKU+nHhJyuJpsalXv0hQq/ttISH4UatGWotkCCHIGayr5iSnfPoV PxGssNUjLX1rSreUAUj7yFQYUv2J/VZlvYsDv1Cq5i5zk8vgGh75KJjV0DO6FQRa kSOY5ElRUJhrd/2vnKVN2feIcF3vOp/WhiP2tcAJG8TFI9U54rSlvTmaJgTIU46i J6Xusouo++TB4+VgFx54hv7/bh7yAPlouQ4iJr/L7aXzKU5U2W71duywP71DXd/5 DtZSqVaQNHcnA+o4tYRbaqfq67GyhE7PYBy2pAQiwDRZOGzPsdqffRvB5S2oyX8N CbfOpI6NhdxQ0Y1EVS1PK24RQnTgS0zrQmyUjld6wTwQBwPQVqRtrkooPVbbMHKF C/xtbFCXx3vek+uYFbFpIfJ0RJXaY1wlSud2Qhk7emxlbI4bZY69N5ZdpYftq3/K Au5qX24+BYZLAcSwVzqf =p7Rh -----END PGP SIGNATURE----- --CE+1k2dSO48ffgeK-- --===============1981678624== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1981678624==--