[Mesa-dev] [PATCH 4/4] configure.ac: check for Glamor requirements only when needed

[Emil Velikov]

On 19 October 2016 at 19:31, Marek Olšák <maraeo at gmail.com> wrote:
> On Wed, Oct 19, 2016 at 2:40 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 18 October 2016 at 23:00, Marek Olšák <maraeo at gmail.com> wrote:
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> ---
>>>  configure.ac | 37 +++++++++++++++++++++++++++----------
>>>  1 file changed, 27 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 12c8165..17dfafd 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -2296,35 +2296,52 @@ dnl Gallium helper functions
>>>  dnl
>>>  gallium_require_llvm() {
>>>      if test "x$MESA_LLVM" = x0; then
>>>          case "$host" in *gnux32) return;; esac
>>>          case "$host_cpu" in
>>>          i*86|x86_64|amd64) AC_MSG_ERROR([LLVM is required to build $1 on x86 and x86_64]);;
>>>          esac
>>>      fi
>>>  }
>>>
>>> -dnl This is for Glamor. Skip this if OpenGL is disabled.
>>> -require_egl_drm() {
>>> +dnl If EGL/X11 or GLX is enabled, make sure they are usable.
>>> +check_glamor_requirements() {
>>
>> With the previous patches you no longer need this, due to the following:
>>  - The correct option is the default one
>>  - If one is missing libgbm.so, libglamoregl.so will fail to load
>> [with decent commit message]
>>  - With recent fix from Chad, we won't advertise the
>> EGL_MESA_platform_gbm extension when EGL is build w/o it.
>>  - The interface between DRI loaders and drivers is stable. So one
>> shouldn't need to rebuild EGL/gbm if they're only interested in the
>> latest fixes in the radeonsi driver.
>>
>> Either way, if you really want this please use something like the following:
>>
>> if test x$enable_egl = xyes; then
>>   case "$with_egl_platforms" in
>>     *drm*)
>>       ;;
>>     *)
>>       AC_MSG_ERROR([--with-egl-platforms=drm is required to build the
>> $1 driver.])
>>       ;;
>>   esac
>> fi
>>
>> One doesn't need any of the enable_opengl, enable_gbm or alike tests.
>> Furthermore glamor relies on the gbm/drm EGL platform so the x11 check
>> is superfluous.
>
> I think there is some misunderstanding.
>
> OpenGL X11/DRI acceleration is enabled by the build system:
> - for GLX by setting $enable_glx = dri
> - for EGL by setting $with_egl_platforms = *x11*
>
> The following code checks if OpenGL on X11/DRI is enabled:
>
>     if test "x$enable_opengl" = xno; then
>         return 0
>     fi
>
>     need_glamor=no
>
>     if test "x$enable_glx" = xdri; then # GLX
>         need_glamor=yes
>     fi
>
>     case "$with_egl_platforms" in # EGL
>         *x11*)
>             need_glamor=yes
>             ;;
>     esac
>
>
IIRC glamor + glx isn't really an option. So as we look the !glamor
case (hmm does newer radeons have acceleration in those cases ?) the
existing generic enable_{dri,glx,opengl} heuristics apply.
If those are broken, please provide the offending configure line so we
can fix this for everyone.

> Once we know that OpenGL on X11/DRI is being built (need_glamor=yes,
> though that's a misnomer), we have to make sure it's usable. This is
> where GBM and EGL/DRM are required. You can't have working OpenGL on
> X11/DRI without those two requirements. Therefore, checks like this
> follow:
>
>         if test "x$enable_gbm" != xyes; then ....
>
>         case "$with_egl_platforms" in
>             *drm*) ...
>
Since enable_gbm is an drm egl_platform requirement it's already
explicitly handled by the drm egl platform detection/selection. Thus
you can drop the former.

> If you don't want X11/DRI support, just disable it. But this patch
> won't let you build radeonsi for X11/DRI without GBM and EGL/DRM.
> There was also the argument that the system-provided libgbm can be
> used, but I have doubts about the stability of its interface.
>
Care to provide some references behind those doubts :-P But seriousl,y
guess I should revive "the deprecate support for DRI driver <> loader
interfaces older than 2-3 years".

As per my earlier comment - I see what you're trying to do here, but
all this chaos would have been avoided in the first place if we've
enabled gbm/platform_drm ages ago. With those toggled, the original
issue that inspired this is very [very] unlikely to happen. Speaking
of which - I cannot find any reports of people (mis)configuring mesa,
leading to lack of glamor/X11 acceleration.

Thanks
Emil