[Buildroot] [PATCH v1 1/2] package/mesa3d: add config option for DRI3 support

Peter Seiderer ps.report at gmx.net
Wed Jun 16 19:54:47 UTC 2021


Hello Arnout,

On Tue, 15 Jun 2021 22:19:11 +0200, Arnout Vandecappelle <arnout at mind.be> wrote:

> On 14/06/2021 23:54, Peter Seiderer wrote:
> > Hello Arnout,
> >
> > On Sun, 13 Jun 2021 11:25:36 +0200, Arnout Vandecappelle <arnout at mind.be> wrote:
> >
> >>  Hi Peter,
> >>
> >> On 13/06/2021 00:30, Peter Seiderer wrote:
> >>> Add config option for DRI3 support and use it instead
> >>> of DRI3 enable/disable logic in *.mk file.
> >>>
> >>> Signed-off-by: Peter Seiderer <ps.report at gmx.net>
> >>> ---
> >>>  package/mesa3d/Config.in |  8 ++++++++
> >>>  package/mesa3d/mesa3d.mk | 12 +++++++-----
> >>>  2 files changed, 15 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/package/mesa3d/Config.in b/package/mesa3d/Config.in
> >>> index d1b3af2054..36acd9758c 100644
> >>> --- a/package/mesa3d/Config.in
> >>> +++ b/package/mesa3d/Config.in
> >>> @@ -16,6 +16,11 @@ menuconfig BR2_PACKAGE_MESA3D
> >>>
> >>>  if BR2_PACKAGE_MESA3D
> >>>
> >>> +config BR2_PACKAGE_MESA3D_DRI3
> >>> +	bool "Enable DRI3 support"
> >>
> >>  Does it make sense to have this as a user-selectable option? Wouldn't it be
> >> better to make this a blind option? (The latter has the advantage that we can
> >> easily refactor it later, without legacy handling and stuff.)
> >
> > As it is an feature option exposed by mesa3d I believe it makes sense...
>
>  I think putting "option exposed by mesa3d" and "make sense" in the same
> sentence is a bit of a stretch :-) The proper thing for mesa3d to do would be to
> enable dri3 automatically if a driver needs it. That we have to pass it
> explicitly is just silly IMHO.
>
>
> >>  I just did a build with just DRI3 selected and it didn't install anything to
> >> target or staging. This suggests that the option isn't useful on its own.
> >
> > Same for numerous other options in buildroot which enable/disable compile time
> > feature include in packages, see e.g. the BR2_PACKAGE_LIBOPENSSL_ENABLE_RC5,
> > BR2_PACKAGE_LIBOPENSSL_ENABLE_RC2, ... and many others...
>
>  BR2_PACKAGE_LIBOPENSSL_ENABLE_RC5 *will* install libcrypto to staging and
> target, so I fail to see your point.

With:

BR2_PACKAGE_OPENSSL=y
BR2_PACKAGE_LIBOPENSSL=y
BR2_PACKAGE_LIBOPENSSL_TARGET_ARCH="linux-aarch64"
# BR2_PACKAGE_LIBOPENSSL_BIN is not set
# BR2_PACKAGE_LIBOPENSSL_ENGINES is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_CHACHA is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_RC5 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_RC2 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_RC4 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_MD2 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_MD4 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_MDC2 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_BLAKE2 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_IDEA is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_SEED is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_DES is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_RMD160 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_WHIRLPOOL is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_BLOWFISH is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_SSL is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_SSL2 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_SSL3 is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_WEAK_SSL is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_PSK is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_CAST is not set
# BR2_PACKAGE_LIBOPENSSL_UNSECURE is not set
# BR2_PACKAGE_LIBOPENSSL_DYNAMIC_ENGINE is not set
# BR2_PACKAGE_LIBOPENSSL_ENABLE_COMP is not set

	$ grep libcrypto build/libopenssl-1.1.1k/.files-list.txt
libopenssl,./usr/lib/libcrypto.so.1.1
libopenssl,./usr/lib/libcrypto.a
libopenssl,./usr/lib/libcrypto.so
libopenssl,./usr/lib/pkgconfig/libcrypto.pc


With:

BR2_PACKAGE_OPENSSL=y
BR2_PACKAGE_LIBOPENSSL=y
BR2_PACKAGE_LIBOPENSSL_TARGET_ARCH="linux-aarch64"
# BR2_PACKAGE_LIBOPENSSL_BIN is not set
# BR2_PACKAGE_LIBOPENSSL_ENGINES is not set
BR2_PACKAGE_LIBOPENSSL_ENABLE_CHACHA=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_RC5=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_RC2=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_RC4=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_MD2=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_MD4=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_MDC2=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_BLAKE2=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_IDEA=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_SEED=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_DES=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_RMD160=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_WHIRLPOOL=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_BLOWFISH=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_SSL=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_SSL2=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_SSL3=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_WEAK_SSL=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_PSK=y
BR2_PACKAGE_LIBOPENSSL_ENABLE_CAST=y
# BR2_PACKAGE_LIBOPENSSL_UNSECURE is not set
# BR2_PACKAGE_LIBOPENSSL_DYNAMIC_ENGINE is not set
BR2_PACKAGE_LIBOPENSSL_ENABLE_COMP=y

	$ grep libcrypto build/libopenssl-1.1.1k/.files-list.txt
libopenssl,./usr/lib/libcrypto.so.1.1
libopenssl,./usr/lib/libcrypto.a
libopenssl,./usr/lib/libcrypto.so
libopenssl,./usr/lib/pkgconfig/libcrypto.pc


I would describe the _ENABLE_... options as enabling/disabling (compile time)
features of libcrypto...

Regards,
Peter


>
>  What I'm saying is: having it as a user-visible option only makes sense if a
> user can do something with that option. Since mesa3d doesn't install anything
> when dri3 is selected, I suspect that it's useless as a user-visible option. And
> as I wrote, making it user-visible has a cost: it means that changing/removing
> the option later becomes more difficult (legacy handling and all that).
>
>  Of course, I could still be wrong. Maybe dri3 does something useful. But to me
> it looks more like the shared-glapi option, that is meaningless by itself but
> that enables a subsystem that is required by some drivers.
>
>
> >>  More importantly: have you checked that DRI3 doesn't have any dependencies of
> >> itself? Check the meson files to see if there are dependencies that are implied
> >> directly by dri3 (i.e., maybe the xshmfence dependency comes from dri3 rather
> >> than the individual dri driver). Those things should be moved around as well,
> >> similar how they are put in BR2_PACKAGE_MESA3D_DRI_DRIVER instead of the
> >> individual drivers.
> >
> > But this mimics the mesa3d internal logic (without an mesa3d exposed feature
> > option)....
>
>  I'm not sure what you mean. What I mean is: in meson.build there is:
>
> if with_platform_x11
> ...
>   if (with_egl or
>       with_dri3 or (
>       with_gallium_vdpau or with_gallium_xvmc or with_gallium_xa or
>       with_gallium_omx != 'disabled'))
>     dep_xcb_xfixes = dependency('xcb-xfixes')
>   endif
>
>
>  This means, IMHO, that in Config.in we need:
>
> config BR2_PACKAGE_MESA3D_DRI3
> 	bool
> 	depends on ...
> 	select BR2_PACKAGE_XLIB_LIBXFIXES if BR2_PACKAGE_XORG7
>
> and there are a bunch more like that.
>
>
>  Regards,
>  Arnout
>
> >
> > Regards,
> > Peter
> >
> >>
> >>
> >>  Series marked as Changes Requested.
> >>
> >>  Regards,
> >>  Arnout
> >>
> >>> +	help
> >>> +	  Enable DRI3 support.
> >>> +
> >>>  # Some Gallium driver needs libelf when built with LLVM support
> >>>  config BR2_PACKAGE_MESA3D_NEEDS_ELFUTILS
> >>>  	bool
> >>> @@ -65,6 +70,8 @@ config BR2_PACKAGE_MESA3D_DRI_DRIVER
> >>>  		!BR2_PACKAGE_MESA3D_OPENGL_GLX && \
> >>>  		!BR2_PACKAGE_MESA3D_OPENGL_EGL && \
> >>>  		!BR2_PACKAGE_MESA3D_OSMESA_GALLIUM
> >>> +	select BR2_PACKAGE_MESA3D_DRI3 if \
> >>> +		(BR2_PACKAGE_XORG7 && BR2_TOOLCHAIN_HAS_SYNC_4)
> >>>  	select BR2_PACKAGE_XLIB_LIBXSHMFENCE if \
> >>>  		(BR2_PACKAGE_XORG7 && BR2_TOOLCHAIN_HAS_SYNC_4)
> >>>
> >>> @@ -359,6 +366,7 @@ config BR2_PACKAGE_MESA3D_VULKAN_DRIVER_INTEL
> >>>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17 # memfd.h
> >>>  	depends on BR2_TOOLCHAIN_USES_GLIBC # ifunc, static_assert
> >>>  	depends on BR2_PACKAGE_XORG7 # xorgproto
> >>> +	select BR2_PACKAGE_MESA3D_DRI3
> >>>  	select BR2_PACKAGE_MESA3D_VULKAN_DRIVER
> >>>  	select BR2_PACKAGE_XORGPROTO
> >>>  	select BR2_PACKAGE_XLIB_LIBXSHMFENCE
> >>> diff --git a/package/mesa3d/mesa3d.mk b/package/mesa3d/mesa3d.mk
> >>> index 5c5f8a33f3..da6e55bf93 100644
> >>> --- a/package/mesa3d/mesa3d.mk
> >>> +++ b/package/mesa3d/mesa3d.mk
> >>> @@ -35,6 +35,12 @@ ifeq ($(BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM),y)
> >>>  MESA3D_CONF_OPTS += -Db_asneeded=false
> >>>  endif
> >>>
> >>> +ifeq ($(BR2_PACKAGE_MESA3D_DRI3),y)
> >>> +MESA3D_CONF_OPTS += -Ddri3=enabled
> >>> +else
> >>> +MESA3D_CONF_OPTS += -Ddri3=disabled
> >>> +endif
> >>> +
> >>>  ifeq ($(BR2_PACKAGE_MESA3D_LLVM),y)
> >>>  MESA3D_DEPENDENCIES += host-llvm llvm
> >>>  MESA3D_MESON_EXTRA_BINARIES += llvm-config='$(STAGING_DIR)/usr/bin/llvm-config'
> >>> @@ -122,13 +128,10 @@ endif
> >>>
> >>>  ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER),)
> >>>  MESA3D_CONF_OPTS += \
> >>> -	-Ddri-drivers= -Ddri3=disabled
> >>> +	-Ddri-drivers=
> >>>  else
> >>>  ifeq ($(BR2_PACKAGE_XLIB_LIBXSHMFENCE),y)
> >>>  MESA3D_DEPENDENCIES += xlib_libxshmfence
> >>> -MESA3D_CONF_OPTS += -Ddri3=enabled
> >>> -else
> >>> -MESA3D_CONF_OPTS += -Ddri3=disabled
> >>>  endif
> >>>  MESA3D_CONF_OPTS += \
> >>>  	-Dshared-glapi=enabled \
> >>> @@ -142,7 +145,6 @@ MESA3D_CONF_OPTS += \
> >>>  else
> >>>  MESA3D_DEPENDENCIES += xlib_libxshmfence
> >>>  MESA3D_CONF_OPTS += \
> >>> -	-Ddri3=enabled \
> >>>  	-Dvulkan-drivers=$(subst $(space),$(comma),$(MESA3D_VULKAN_DRIVERS-y))
> >>>  endif
> >>>
> >>>
> >




More information about the buildroot mailing list