[Buildroot] [PATCH] package/mesa3d: vulkan with x11 needs dri3

[Buildroot] [PATCH] package/mesa3d: vulkan with x11 needs dri3

[Yann E. MORIN]

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

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Bernd Kuhls <bernd.kuhls@t-online.de>
Cc: Romain Naour <romain.naour@gmail.com>
---
 package/mesa3d/Config.in | 3 +++
 package/mesa3d/mesa3d.mk | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/mesa3d/Config.in b/package/mesa3d/Config.in
index eec72654cd..06574ae513 100644
--- a/package/mesa3d/Config.in
+++ b/package/mesa3d/Config.in
@@ -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
 
diff --git a/package/mesa3d/mesa3d.mk b/package/mesa3d/mesa3d.mk
index 5c01f92e22..f82facf7d1 100644
--- a/package/mesa3d/mesa3d.mk
+++ b/package/mesa3d/mesa3d.mk
@@ -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
-- 
2.14.1

[Buildroot] [PATCH] package/mesa3d: vulkan with x11 needs dri3

[Thomas Petazzoni]

Hello Yann,

On Thu, 23 Aug 2018 11:08:27 +0200, Yann E. MORIN wrote:
> Fixes:
>     http://autobuild.buildroot.org/results/356/35653fc6f57dc169a8aae6baedb1acd1049b50ec/
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Bernd Kuhls <bernd.kuhls@t-online.de>
> Cc: Romain Naour <romain.naour@gmail.com>

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


> -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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/mesa3d: vulkan with x11 needs dri3

[Yann E. MORIN]

Thomas, All,

On 2018-08-23 12:11 +0200, Thomas Petazzoni spake thusly:
> On Thu, 23 Aug 2018 11:08:27 +0200, Yann E. MORIN wrote:
> > Fixes:
> >     http://autobuild.buildroot.org/results/356/35653fc6f57dc169a8aae6baedb1acd1049b50ec/
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Bernd Kuhls <bernd.kuhls@t-online.de>
> > Cc: Romain Naour <romain.naour@gmail.com>
> 
> I was about to apply, but something caught my eye.

Dang...

> > -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).

Hmm...

> 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...

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

Probably, yes...

> 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.

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

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] package/mesa3d: vulkan with x11 needs dri3

[Thomas Petazzoni]

Hello,

On Thu, 23 Aug 2018 12:20:29 +0200, Yann E. MORIN wrote:

> > 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...

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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/mesa3d: vulkan with x11 needs dri3

[Yann E. MORIN]

Thomas, All,

On 2018-08-23 13:23 +0200, Thomas Petazzoni spake thusly:
> On Thu, 23 Aug 2018 12:20:29 +0200, Yann E. MORIN wrote:
> > > 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...
> 
> 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.
> So, perhaps the Buildroot packaging can be improved (it would be
> nice!), but it will remain anyway a complex package.
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'