package/mesa3d: vulkan with x11 needs dri3

PATCH New Yann E. MORIN 2 participants 5 messages Aug 23, 2018

lore

Share & tools

lore

Yann E. MORIN

author

2018-08-23 09:08 UTC #0 lore raw

PATCH 1/1 package/mesa3d: vulkan with x11 needs dri3

Fixes:

    http://autobuild.buildroot.org/results/356/35653fc6f57dc169a8aae6baedb1acd1049b50ec/

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Bernd Kuhls <[email protected]>

Cc Romain Naour <[email protected]>

---
 package/mesa3d/Config.in | 3 +++
 package/mesa3d/mesa3d.mk | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

package/mesa3d/Config.in +3 -0

index eec72654cd..06574ae513 100644
@@ -210,16 +210,19 @@ comment "Vulkan drivers"
 config BR2_PACKAGE_MESA3D_VULKAN_DRIVER_INTEL
 	bool "Vulkan Intel driver"
 	depends on BR2_i386 || BR2_x86_64
+	depends on BR2_TOOLCHAIN_HAS_SYNC_4 # libxshmfence
 	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_18 # memfd.h
 	depends on BR2_TOOLCHAIN_USES_GLIBC # ifunc, static_assert
 	depends on BR2_PACKAGE_XORG7 # xorgproto
 	select BR2_PACKAGE_MESA3D_VULKAN_DRIVER
 	select BR2_PACKAGE_XORGPROTO
+	select BR2_PACKAGE_XLIB_LIBXSHMFENCE
 	help
 	  Vulkan driver for Intel hardware from Ivy Bridge onward.
 
 comment "intel vulkan depends on X.org and needs a glibc toolchain w/ headers >= 3.18"
 	depends on BR2_i386 || BR2_x86_64
+	depends on BR2_TOOLCHAIN_HAS_SYNC_4
 	depends on !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_18 || \
 		!BR2_TOOLCHAIN_USES_GLIBC || !BR2_PACKAGE_XORG7

package/mesa3d/mesa3d.mk +1 -1

index 5c01f92e22..f82facf7d1 100644
@@ -107,7 +107,7 @@ MESA3D_CONF_OPTS += \
 	--enable-gallium-extra-hud
 endif
 
-ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER),)
+ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER)$(BR2_PACKAGE_MESA3D_VULKAN_DRIVER),)
 MESA3D_CONF_OPTS += \
 	--without-dri-drivers --disable-dri3
 else

Signature · 1 line

2.14.1

Thomas Petazzoni

2018-08-23 10:11 UTC #1 lore raw

PATCH 1/1 package/mesa3d: vulkan with x11 needs dri3

Hello Yann,

Quoted 6 lines · Yann E. MORIN

Fixes:
    http://autobuild.buildroot.org/results/356/35653fc6f57dc169a8aae6baedb1acd1049b50ec/
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Bernd Kuhls <[email protected]>
Cc Romain Naour <[email protected]>

I was about to apply, but something caught my eye.

package/mesa3d/mesa3d.mk MESA3D_CONF_OPTS += \ · line 112

Quoted 4 lines

package/mesa3d/mesa3d.mk +1 -1

-ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER),)
+ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER)$(BR2_PACKAGE_MESA3D_VULKAN_DRIVER),)
 MESA3D_CONF_OPTS += \
 	--without-dri-drivers --disable-dri3

This change makes sure that when BR2_PACKAGE_MESA3D_VULKAN_DRIVER=y, we no longer pass --disable-dri3, because we indeed what DRI3 to be enabled for the Intel Vulkan driver to build.

However, as a side-effect, it is also going to cause --without-dri-drivers to no longer be passed, even though we are possibly not building any DRI driver (ie BR2_PACKAGE_MESA3D_DRI_DRIVER may be disabled).

Doesn't this requires a bit more rework to be clean ?

Perhaps we need to separate the handling of enable-dri3/disable-dri3 from the with-dri-drivers/without-dri-drivers ?

Side question: the error message "ANV Vulkan driver requires DRI3 when built with X11" implies that the Intel Vulkan driver can be built without X11, while in Buildroot we require X11 to build the Intel Vulkan driver. Note: this is unrelated to the build failure, and could be improved as a separate patch, but is definitely not mandatory to fix. It's just something I noticed while looking at this issue.

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Yann E. MORIN

author

2018-08-23 10:20 UTC #2 lore raw

PATCH 1/1 package/mesa3d: vulkan with x11 needs dri3

Thomas, All,

On 2018-08-23 12:11 +0200, Thomas Petazzoni spake thusly:

Quoted 9 lines · Yann E. MORIN

Quoted 6 lines
Fixes:
    http://autobuild.buildroot.org/results/356/35653fc6f57dc169a8aae6baedb1acd1049b50ec/
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Bernd Kuhls <[email protected]>
Cc Romain Naour <[email protected]>
I was about to apply, but something caught my eye.

Dang...

Quoted 9 lines

package/mesa3d/mesa3d.mk MESA3D_CONF_OPTS += \ · line 112
Quoted 4 lines
package/mesa3d/mesa3d.mk +1 -1
-ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER),)
+ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER)$(BR2_PACKAGE_MESA3D_VULKAN_DRIVER),)
 MESA3D_CONF_OPTS += \
 	--without-dri-drivers --disable-dri3
This change makes sure that when BR2_PACKAGE_MESA3D_VULKAN_DRIVER=y, we no longer pass --disable-dri3, because we indeed what DRI3 to be enabled for the Intel Vulkan driver to build.

Quoted 4 lines

However, as a side-effect, it is also going to cause --without-dri-drivers to no longer be passed, even though we are possibly not building any DRI driver (ie BR2_PACKAGE_MESA3D_DRI_DRIVER may be disabled).

Hmm...

Quoted 1 line

Doesn't this requires a bit more rework to be clean ?

Yes, I'll have a further look by the end of the day. I've marked it as Changes Requested.

To be hnest, the whole of mesa3d may need a complete overhaul I think. Theree dubious constructs here and there, like:

    config BR2_PACKAGE_MESA3D_DRI_DRIVER
        bool
        select BR2_PACKAGE_MESA3D_DRIVER
        select BR2_PACKAGE_XLIB_LIBXSHMFENCE if \
                (BR2_PACKAGE_XORG7 && BR2_TOOLCHAIN_HAS_SYNC_4)

So now I wonder if we shouldn't have something akin to this for Vulkan.

Really, mesa3d has become quite a nest of snakes. It's hard to follow and understand the dependencies between each config options...

Quoted 2 lines

Perhaps we need to separate the handling of enable-dri3/disable-dri3 from the with-dri-drivers/without-dri-drivers ?

Probably, yes...

Quoted 6 lines

Side question: the error message "ANV Vulkan driver requires DRI3 when built with X11" implies that the Intel Vulkan driver can be built without X11, while in Buildroot we require X11 to build the Intel Vulkan driver. Note: this is unrelated to the build failure, and could be improved as a separate patch, but is definitely not mandatory to fix. It's just something I noticed while looking at this issue.

I'll have a look later today.

Thanks for the feedback. :-)

Regards,
Yann E. MORIN.

Quoted 8 lines

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Thomas Petazzoni

2018-08-23 11:23 UTC #3 lore raw

PATCH 1/1 package/mesa3d: vulkan with x11 needs dri3

Hello,

Quoted 14 lines · Yann E. MORIN

Quoted 1 line
Doesn't this requires a bit more rework to be clean ?
Yes, I'll have a further look by the end of the day. I've marked it as Changes Requested.
To be hnest, the whole of mesa3d may need a complete overhaul I think. Theree dubious constructs here and there, like:
    config BR2_PACKAGE_MESA3D_DRI_DRIVER
        bool
        select BR2_PACKAGE_MESA3D_DRIVER
        select BR2_PACKAGE_XLIB_LIBXSHMFENCE if \
                (BR2_PACKAGE_XORG7 && BR2_TOOLCHAIN_HAS_SYNC_4)

Quoted 4 lines

So now I wonder if we shouldn't have something akin to this for Vulkan.
Really, mesa3d has become quite a nest of snakes. It's hard to follow and understand the dependencies between each config options...

Yes, the mesa3d package is quite complicated. But I believe this (at least partially) reflects that the mesa3d configuration itself is quite complicated. There are lots of drivers, all having special dependencies.

So, perhaps the Buildroot packaging can be improved (it would be nice!), but it will remain anyway a complex package.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Yann E. MORIN

author

2018-08-23 14:36 UTC #4 lore raw

PATCH 1/1 package/mesa3d: vulkan with x11 needs dri3

Thomas, All,

On 2018-08-23 13:23 +0200, Thomas Petazzoni spake thusly:

Quoted 15 lines · Yann E. MORIN

Quoted 14 lines
Quoted 1 line
Doesn't this requires a bit more rework to be clean ?
Yes, I'll have a further look by the end of the day. I've marked it as Changes Requested.
To be hnest, the whole of mesa3d may need a complete overhaul I think. Theree dubious constructs here and there, like:
    config BR2_PACKAGE_MESA3D_DRI_DRIVER
        bool
        select BR2_PACKAGE_MESA3D_DRIVER
        select BR2_PACKAGE_XLIB_LIBXSHMFENCE if \
                (BR2_PACKAGE_XORG7 && BR2_TOOLCHAIN_HAS_SYNC_4)

Quoted 8 lines

Quoted 4 lines
So now I wonder if we shouldn't have something akin to this for Vulkan.
Really, mesa3d has become quite a nest of snakes. It's hard to follow and understand the dependencies between each config options...
Yes, the mesa3d package is quite complicated. But I believe this (at least partially) reflects that the mesa3d configuration itself is quite complicated. There are lots of drivers, all having special dependencies.

Yes, but ciurrently, I'm afraid we can end up with an inconsistent .config.

That snippet:

    config BR2_PACKAGE_MESA3D_DRI_DRIVER
        bool
        select BR2_PACKAGE_MESA3D_DRIVER
        select BR2_PACKAGE_XLIB_LIBXSHMFENCE if \
                (BR2_PACKAGE_XORG7 && BR2_TOOLCHAIN_HAS_SYNC_4)

    config BR2_PACKAGE_MESA3D_DRI_DRIVER_NOUVEAU
        bool "DRI nouveau driver"
        select BR2_PACKAGE_MESA3D_DRI_DRIVER
        select BR2_PACKAGE_LIBDRM_NOUVEAU

Means that, if using Xorg and a toolchain that does not have sync-4, and nouveau is enabled, then xlib-libxshnfence is not forcully selected.

So, in the .mk:

    ifeq ($(BR2_PACKAGE_MESA3D_DRI_DRIVER),)
        [--SNIP--]
    else
    ifeq ($(BR2_PACKAGE_XLIB_LIBXSHMFENCE),y)
    MESA3D_DEPENDENCIES += xlib_libxshmfence
    MESA3D_CONF_OPTS += --enable-dri3
    else
    MESA3D_CONF_OPTS += --disable-dri3
    endif
        [--SNIP--]
    endif

Which means we're going to actually disable dri3, even though at least one DRI driver has been enabled...

It turns out that the only archs Buildroot knows of, that don't have sync-4, also do not have sync-1. And mesa3d requires sync-1.

Still, the construct in the Config.in really looks suspicious. The sync4 dependency is from xlib-libxshmfence. I.e. we can select DRI drivers even if dri3 is not available because xlib-libxshmfence is not availabel because sync-4 is not. It really looks incorrect... :-/

Bernd, Romain, can you please comment on this?

Regards,
Yann E. MORIN.

Quoted 8 lines

So, perhaps the Buildroot packaging can be improved (it would be nice!), but it will remain anyway a complex package.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'