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

[Emil Velikov]

On 24 October 2016 at 18:21, Marek Olšák <maraeo at gmail.com> wrote:
> On Mon, Oct 24, 2016 at 11:33 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> 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.
>
> That's not what it means. It's not about Glamor. If I rename
> "need_glamor" to "need_opengl_in_X", will it make more sense? Glamor
> is a requirement for OpenGL in X (GLX), so in order to support GLX, we
> need Glamor, thus we need EGL/DRM. That's the dependency chain.
>
> To make it clear:
> - GLX depends on X acceleration.
> - EGL/X11 also depends on X acceleration.
Indeed, and people may want the swrast 'acceleration' for GLX and
radeon one for EGL/X11 and vice-versa.
Regardless, what gets build and shipped is packaging/distribution decision.

> - X acceleration depends on Glamor.
Glamor is one way to provide X acceleration.

> - Glamor depends on EGL/DRM and GBM.
>
Glamor may depend on GBM. Since radeons/mesa drivers rely on
Glamor/EGL to be GBM aware they need EGL/DRM. The latter already
depends on GBM so we can drop the enable_gbm check.

> Thus, if you enable GLX or EGL/X11, you also need EGL/DRM and GBM.
>
> Is it clear now?
>
Guess it's the opposite way - I'm not clear enough :-\

We're enforcing packaging/distribution decisions even if people don't
need them. We can devote our focus/energy to a) toggle things to sane
defaults (thanks for that) and b) have comprehensive messages as
people unintentionally, or not shoot themselves in the foot.

Please ?

Thanks
Emil