From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Foss Subject: Re: [PATCH v2 hwc] drm_hwcomposer: provide a common gralloc handle definition Date: Tue, 31 Oct 2017 20:14:51 +0100 Message-ID: <1509477291.17694.2.camel@collabora.com> References: <20171031145920.20343-1-robh@kernel.org> <20171031164425.cmhzi25zmrbvxntb@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1756886043==" Return-path: Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9AA856E631 for ; Tue, 31 Oct 2017 19:14:55 +0000 (UTC) In-Reply-To: <20171031164425.cmhzi25zmrbvxntb@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter , Rob Herring Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1756886043== Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-cjzSPGD7j4bhIpwdHuen" --=-cjzSPGD7j4bhIpwdHuen Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2017-10-31 at 17:44 +0100, Daniel Vetter wrote: > On Tue, Oct 31, 2017 at 09:59:20AM -0500, Rob Herring wrote: > > EGL, gralloc, and HWC must all have a common definition of fd's and > > int's > > in native_handle_t to share the fd and width, height, format, etc. > > of a > > dmabuf. > >=20 > > Move the definition into HWC so we aren't dependent on a specific > > gralloc > > implementation and so we don't have to create an importer just for > > different native_handle_t layouts. This will allow supporting > > multiple > > gralloc implementations that conform to this layout. > >=20 > > For now, this is aligned with gbm_gralloc's struct. Once we change > > gbm_gralloc and mesa to point to this copy, we can make > > modifications to > > the struct. > >=20 > > Signed-off-by: Rob Herring > > --- > > Android.mk | 1 - > > gralloc_drm_handle.h | 87 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 87 insertions(+), 1 deletion(-) > > create mode 100644 gralloc_drm_handle.h > >=20 > > diff --git a/Android.mk b/Android.mk > > index 8b11e375adde..ee5b8bfb44d0 100644 > > --- a/Android.mk > > +++ b/Android.mk > > @@ -47,7 +47,6 @@ LOCAL_SHARED_LIBRARIES :=3D \ > > LOCAL_STATIC_LIBRARIES :=3D libdrmhwc_utils > > =20 > > LOCAL_C_INCLUDES :=3D \ > > - external/drm_gralloc \ > > system/core/libsync > > =20 > > LOCAL_SRC_FILES :=3D \ > > diff --git a/gralloc_drm_handle.h b/gralloc_drm_handle.h > > new file mode 100644 > > index 000000000000..e2f35dda539b > > --- /dev/null > > +++ b/gralloc_drm_handle.h > > @@ -0,0 +1,87 @@ > > +/* > > + * Copyright (C) 2010-2011 Chia-I Wu > > + * Copyright (C) 2010-2011 LunarG Inc. > > + * Copyright (C) 2016-2017 Linaro, Ltd., Rob Herring > org> > > + * > > + * Permission is hereby granted, free of charge, to any person > > obtaining a > > + * copy of this software and associated documentation files (the > > "Software"), > > + * to deal in the Software without restriction, including without > > limitation > > + * the rights to use, copy, modify, merge, publish, distribute, > > sublicense, > > + * and/or sell copies of the Software, and to permit persons to > > whom the > > + * Software is furnished to do so, subject to the following > > conditions: > > + * > > + * The above copyright notice and this permission notice shall be > > included > > + * in all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > > EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > > EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > > DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > > ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > > OTHER > > + * DEALINGS IN THE SOFTWARE. > > + */ > > + > > +#ifndef _GRALLOC_DRM_HANDLE_H_ > > +#define _GRALLOC_DRM_HANDLE_H_ > > + > > +#include > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +struct gralloc_drm_handle_t { > > + native_handle_t base; > > + > > + /* file descriptors */ > > + int prime_fd; > > + > > + /* integers */ > > + int magic; > > + > > + int width; > > + int height; > > + int format; > > + int usage; > > + > > + int name; /* the name of the bo */ > > + int stride; /* the stride in bytes */ > > + int data_owner; /* owner of data (for validation) */ > > + > > + uint64_t modifier __attribute__((aligned(8))); /* buffer > > modifiers */ > > + union { > > + void *data; /* private pointer for gralloc */ > > + uint64_t reserved; > > + } __attribute__((aligned(8))); > > +}; > > +#define GRALLOC_HANDLE_MAGIC 0x5f47424d >=20 > Maybe add a >=20 > #define GRALLOC_HANDLE_VERSION 1 > #define GRALLOC_HANDLE_MAGIC (0x5f47424d + GRALLOC_HANDLE_VERSION) >=20 > and a comment that the version needs to be incremented for every > change, > to catch mismatching mesa/hwc versions? Given the layout there's no > way to > add more fds in a backwards-compatible way, so we need full > versioning I > think, and treat any change as a breaking one. > -Daniel This seems like a good idea to me. With this change feel free to add my r-b. >=20 > > +#define GRALLOC_HANDLE_NUM_INTS ( =09 > > \ > > + ((sizeof(struct gralloc_drm_handle_t) - > > sizeof(native_handle_t))/sizeof(int)) \ > > + - GRALLOC_HANDLE_NUM_FDS) > > + > > +static inline struct gralloc_drm_handle_t > > *gralloc_drm_handle(buffer_handle_t _handle) > > +{ > > + struct gralloc_drm_handle_t *handle =3D > > + (struct gralloc_drm_handle_t *) _handle; > > + > > + if (handle && (handle->base.version !=3D sizeof(handle- > > >base) || > > + handle->base.numInts !=3D > > GRALLOC_HANDLE_NUM_INTS || > > + handle->base.numFds !=3D > > GRALLOC_HANDLE_NUM_FDS || > > + handle->magic !=3D GRALLOC_HANDLE_MAGIC)) > > + return NULL; > > + > > + return handle; > > +} > > + > > +static inline int gralloc_drm_get_prime_fd(buffer_handle_t > > _handle) > > +{ > > + struct gralloc_drm_handle_t *handle =3D > > gralloc_drm_handle(_handle); > > + return (handle) ? handle->prime_fd : -1; > > +} > > + > > +#ifdef __cplusplus > > +} > > +#endif > > +#endif /* _GRALLOC_DRM_HANDLE_H_ */ > > --=20 > > 2.14.1 > >=20 > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >=20 >=20 --=-cjzSPGD7j4bhIpwdHuen Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQIcBAABCAAGBQJZ+MurAAoJELeaC2oR7vcnMUkQAJAL6MOq7aDxIeqOVmdHckXS qNDOqwV4pz3hUwAfjZ36Sftme0bMJxNjpAXGsmU/HKqbI5Lx2+PH25g4qFpw02u8 iLk8h5c8PaItLD9eGIPYNXniYsaWSfPj1AMo5NPbZ6RTaLBPTWDKXjBxpAty52JI 2nafNERWq6I1g7B9MITb81r7nCT5+Ik0JyRtBLAwgwotx5P5TsMtOw50lypxTPbC iipN8HJlQKHconsXdXLm3pyeFuFWrcqv0qCEj417gvpOy6IQIt5Xl5pDvzLOYEyg 4yG2zPhTDD45CV+kLs+Uh41PxOpics2+AsKW8TaH9pPwDcAD5bkPxAD0W/LmbrXv TKoxTITaekIVwldIIVOB2sMAqXVhb/NmWnxAM/Pt+qiFJMk7jgwq33slyPY+VPgE LF1WvMhiya18eCyTh5DlrX1uO4bR8+W2rfsNI8uGb35Z938GKmJPJKxbz4nWE74w Ye783gyGlCzJt3OT6MOblHgIL2f77i9AcWOtrCBb+BXtalRApIFActqWUDnupt1A KOnQELDZehMO0yrcgsueafSE7JiOGfHTC0aTZtMvFX60iE5xAjRlhG/dS81NVv// v6zFNoJdWK+hF1sO11MEx5EKqVkezrRS+XPVJR1FqOVft+9iKzciC+qBc4blmzRq hXLA3wXijdcoN71HHehc =hePl -----END PGP SIGNATURE----- --=-cjzSPGD7j4bhIpwdHuen-- --===============1756886043== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1756886043==--