From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Roper Subject: Re: [PATCH 2/4] drm: Add plane type property Date: Thu, 27 Feb 2014 15:24:08 -0800 Message-ID: <20140227232408.GU27672@intel.com> References: <1393539283-5901-1-git-send-email-matthew.d.roper@intel.com> <1393539283-5901-3-git-send-email-matthew.d.roper@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 7DC79FB9E5 for ; Thu, 27 Feb 2014 15:23:34 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Rob Clark Cc: "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org On Thu, Feb 27, 2014 at 05:39:00PM -0500, Rob Clark wrote: > On Thu, Feb 27, 2014 at 5:14 PM, Matt Roper wrote: > > Add a plane type property to allow userspace to distinguish > > sprite/overlay planes from primary planes. In the future we may extend > > this to cover cursor planes as well. > > > > Signed-off-by: Matt Roper > > --- > > drivers/gpu/drm/drm_crtc.c | 32 ++++++++++++++++++++++++++++++++ > > include/drm/drm_crtc.h | 1 + > > include/uapi/drm/drm_mode.h | 3 +++ > > 3 files changed, 36 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index 21c6d4b..1032eaf 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -114,6 +114,14 @@ static const struct drm_prop_enum_list drm_dpms_enum_list[] = > > > > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list) > > > > +static const struct drm_prop_enum_list drm_plane_type_enum_list[] = > > +{ > > + { DRM_MODE_PLANE_TYPE_SPRITE, "Sprite" }, > > I'm not the *hugest* fan of using the name "sprite".. at least that > too me implies sort of a subset of possible functionality of a plane.. Any suggestions on a better name? Maybe call them "traditional" planes and then just give new names to the other types (primary, cursor) that we wind up exposing when appropriate client caps are set? > > > + { DRM_MODE_PLANE_TYPE_PRIMARY, "Primary" }, > > +}; > > + > > +DRM_ENUM_NAME_FN(drm_get_plane_type, drm_plane_type_enum_list) > > + > > /* > > * Optional properties > > */ > > @@ -1046,6 +1054,10 @@ int drm_plane_init(struct drm_device *dev, struct drm_plane *plane, > > INIT_LIST_HEAD(&plane->head); > > } > > > > + drm_object_attach_property(&plane->base, > > + dev->mode_config.plane_type_property, > > + DRM_MODE_PLANE_TYPE_SPRITE); > > + > > out: > > drm_modeset_unlock_all(dev); > > > > @@ -1114,6 +1126,10 @@ int drm_plane_set_primary(struct drm_device *dev, struct drm_plane *plane, > > > fwiw, this comment probably belongs in #1/4 but: > > you probably don't need to introduce drm_plane_set_primary().. > instead you could just rename the 'bool priv' to 'bool prim'. I think > there are just three drivers using primary planes.. I'm not 100% sure > about exynos, but both omap and msm, the private plane == primary > plane. At least it was the intention to morph that into primary > planes. I'd like to handle cursors with this eventually as well, so I'm not sure whether just changing the meaning of priv by itself will get us everything we need. It seems like we probably need to provide a whole lot more information about the capabilities and limitations of each plane at drm_plane_init() and then expose those all as plane properties so that userspace knows what it can and can't do. In theory we could expose cursor planes exactly the same way we expose "traditional" planes today as long as we made sufficient plane properties available to userspace to describe the min/max size limitations and such. > > Anyways, other than that I like the patchset. Hopefully I should get > to rebasing the atomic patches real soon now, so I'll try rebasing on > top of this and see how it goes. > > BR, > -R Sounds good; thanks for the review. Matt -- Matt Roper Graphics Software Engineer ISG Platform Enabling & Development Intel Corporation (916) 356-2795