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

[Emil Velikov]

On 24 October 2016 at 19:28, Marek Olšák <maraeo at gmail.com> wrote:
> On Mon, Oct 24, 2016 at 7:41 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> 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.
>
> It's the *only* way to provide X acceleration on radeonsi.
>
>>
>>> - 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.
>
> BTW, xf86-video-amdgpu also depends on GBM. I think it's the only
> vendor-specific DDX that uses GBM. I can remove the check if EGL/DRM
> requires GBM already.
>
>>
>>> 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 ?
>
> OK, I'll give you an entirely different point of view:
>
> If EGL/DRM and EGL/surfaceless are disabled at the same time,
> radeonsi_dri.so is unusable. It can't be used by any API or window
> system whatsoever. It's a useless binary that can't do anything and
> only takes space. Why would anyone want to build that?
>
> EGL/surfaceless enables users to use radeonsi_dri.so without any window system.
>
One could also have EGL/android in a container within CrOS. Admittedly
I doubt anyone has tried it, so we'll think about it at a later stage.

> EGL/drm enables everything else: X, Wayland, KMS offscreen rendering.
> Since it enables X, it indirectly enables everything that works inside
> X: OpenGL (GLX, EGL/X11), VDPAU, etc.
>
> I'm gonna simplify the function based on that.
>
Exactly what I was saying: if we want some security, keep the top
level dependency - egl/drm (or egl/surfaceless even) but don't track
any of the implicit ones.

Thanks again !
Emil