On Tue, 10 Jul 2018 08:41:46 +0100 Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Pekka, > > On Mon, 29 Jan 2018 at 15:01, Pekka Paalanen <ppaala...@gmail.com> wrote: > > On Wed, 20 Dec 2017 12:26:57 +0000 Daniel Stone <dani...@collabora.com> > > wrote: > > > @@ -212,6 +212,9 @@ if test x$enable_drm_compositor = xyes; then > > > PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78], > > > [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports > > > atomic API])], > > > [AC_MSG_WARN([libdrm does not support atomic > > > modesetting, will omit that capability])]) > > > + PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83], > > > + [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports > > > modifier advertisement])], > > > + [AC_MSG_WARN([libdrm does not support modifier > > > advertisement])]) > > > PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2], > > > [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports > > > import with modifiers])], > > > [AC_MSG_WARN([GBM does not support dmabuf import, will > > > omit that capability])]) > > > > Hi, > > > > I wonder, we are getting more and more of these "libdrm has ..." > > feature checks. How about merging some to produce fewer build types? > > Just a general idea, not specifically for this patch. > > Assuming enough distros support the new bits, I'd be open to having > two levels of libdrm support: ancient or supports-everything. > Annoyingly the gbm check is separate, but I suppose we could connect, > e.g., both of libdrm >= 2.4.83 + gbm >= 17.2 to modifiers, rather than > having modifiers half-supported when only one of them is sufficiently > new. That sounds reasonable to me. Now that we've heard of the weak functions trick, I'd be happy to remove some autoconf checks for library functions and replace them with runtime weak function checks that would also be reported in the log. Maybe that can cut the number of #ifdefs and checks. Let's keep these ideas for follow-up. They would help the Meson migration as well, having less dependency checks to cross-check. > > > + unsigned i, j; > > > + drmModePropertyBlobRes *blob; > > > + struct drm_format_modifier_blob *fmt_mod_blob; > > > + uint32_t *blob_formats; > > > + struct drm_format_modifier *blob_modifiers; > > > + > > > + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id); > > > + if (!blob) > > > + return false; > > > + > > > + fmt_mod_blob = blob->data; > > > > Should we not check that fmt_mod_blob.version == 1? > > No. Future versions can add new bits to the end of the blob, but they > can't break compatibility - doing so would require a new property > name. Usually DRM uses size to do this, but as we have two > variable-length arrays whose size can't be trivially determined by the > client, we use version to indicate any possible future extensions to > the structure, and the offset fields to point to the VLAs. > > The current version is 1; I guess we could check for version 0 and > bail out if we see it, but no kernel has ever had it and it seems a > bit pathological. Future versions will still be parseable as the > original, so those are fine. Understood. Thanks, pq
pgp_1lW0E5AnD.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
