From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v1 1/2] package/mesa3d: add config option for DRI3 support
Date: Tue, 15 Jun 2021 22:19:11 +0200 [thread overview]
Message-ID: <091fecfd-f83c-291c-76ed-75d3b2a496d6@mind.be> (raw)
In-Reply-To: <20210614235410.636c5e27@gmx.net>
On 14/06/2021 23:54, Peter Seiderer wrote:
> Hello Arnout,
>
> On Sun, 13 Jun 2021 11:25:36 +0200, Arnout Vandecappelle <arnout@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@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.
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
>>>
>>>
>
next prev parent reply other threads:[~2021-06-15 20:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-12 22:30 [Buildroot] [PATCH v1 1/2] package/mesa3d: add config option for DRI3 support Peter Seiderer
2021-06-12 22:30 ` [Buildroot] [PATCH v1 2/2] package/mesa3d: gallium/kmsro drivers require dri3 for X11 Peter Seiderer
2021-06-13 9:33 ` Arnout Vandecappelle
2021-06-14 21:47 ` Peter Seiderer
2021-06-13 9:25 ` [Buildroot] [PATCH v1 1/2] package/mesa3d: add config option for DRI3 support Arnout Vandecappelle
2021-06-14 21:54 ` Peter Seiderer
2021-06-15 20:19 ` Arnout Vandecappelle [this message]
2021-06-16 19:54 ` Peter Seiderer
2021-06-16 21:50 ` Arnout Vandecappelle
2021-06-16 22:21 ` Peter Seiderer
2021-12-17 21:00 ` Arnout Vandecappelle
2021-12-19 7:16 ` Michael Taubert
[not found] ` <c8c49a7c-fea0-c96c-ba23-4ff17a1c0d3e@arachnodroid.de>
2021-12-19 7:22 ` Michael Taubert
2021-12-20 22:00 ` Arnout Vandecappelle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=091fecfd-f83c-291c-76ed-75d3b2a496d6@mind.be \
--to=arnout@mind.be \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.