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

[Marek Olšák]

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.

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.

Marek