[Buildroot] [PATCH v1 1/2] package/mesa3d: add config option for DRI3 support
Arnout Vandecappelle
arnout at mind.be
Wed Jun 16 21:50:41 UTC 2021
On 16/06/2021 21:54, Peter Seiderer wrote:
> 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...
Exactly: by enabling the RC4 option, something additional gets built into
libcrypto. But I think the dri3 option doesn't really enable anything at all...
- dri3 enabled but none of the drivers using it enabled: has no effect on what
is installed to taret/staging (i.e. anything that does get installed is exactly
the same as without that optoin.
- dri3 not enabled but one of the drivers using it is enabled: build fails.
I'm not sure of this, but I suspect this is the case. that's why I suspect that
it makes no sense as a user-visible option:
>> 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).
Regards,
Arnout
>>
>> 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