Re: [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  wrote:
> On 24 October 2016 at 18:21, Marek Olšák  wrote:
>> On Mon, Oct 24, 2016 at 11:33 AM, Emil Velikov  
>> wrote:
>>> On 19 October 2016 at 19:31, Marek Olšák  wrote:
 On Wed, Oct 19, 2016 at 2:40 PM, Emil Velikov  
 wrote:
> On 18 October 2016 at 23:00, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> ---
>>  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 

Re: [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  wrote:
> On Mon, Oct 24, 2016 at 7:41 PM, Emil Velikov  
> wrote:
>> On 24 October 2016 at 18:21, Marek Olšák  wrote:
>>> On Mon, Oct 24, 2016 at 11:33 AM, Emil Velikov  
>>> wrote:
 On 19 October 2016 at 19:31, Marek Olšák  wrote:
> On Wed, Oct 19, 2016 at 2:40 PM, Emil Velikov  
> wrote:
>> On 18 October 2016 at 23:00, Marek Olšák  wrote:
>>> From: Marek Olšák 
>>>
>>> ---
>>>  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 

Re: [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  wrote:
> On Mon, Oct 24, 2016 at 11:33 AM, Emil Velikov  
> wrote:
>> On 19 October 2016 at 19:31, Marek Olšák  wrote:
>>> On Wed, Oct 19, 2016 at 2:40 PM, Emil Velikov  
>>> wrote:
 On 18 October 2016 at 23:00, Marek Olšák  wrote:
> From: Marek Olšák 
>
> ---
>  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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

On Mon, Oct 24, 2016 at 11:33 AM, Emil Velikov  wrote:
> On 19 October 2016 at 19:31, Marek Olšák  wrote:
>> On Wed, Oct 19, 2016 at 2:40 PM, Emil Velikov  
>> wrote:
>>> On 18 October 2016 at 23:00, Marek Olšák  wrote:
 From: Marek Olšák 

 ---
  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.
- X acceleration depends on Glamor.
- Glamor depends on EGL/DRM and GBM.

Thus, if you enable GLX or EGL/X11, you also need EGL/DRM and GBM.

Is it clear now?

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Emil Velikov]

On 24 October 2016 at 10:47, Michel Dänzer  wrote:
> On 24/10/16 06:33 PM, Emil Velikov wrote:
>>
>> IIRC glamor + glx isn't really an option.
>
> That's true for Xorg, because Xorg is the thing which provides GLX in
> the first place, so it cannot use GLX itself. (glamor on GLX works in
> Xephyr though)
>
>
>> So as we look the !glamor case (hmm does newer radeons have
>> acceleration in those cases ?)
>
> Not in X.
>
Precisely. So the x11 egl platform requirement seem artificial since
it has nothing to do with acceleration (making things run). To make it
even funnier - if one wants X-less setup, that won't be possible since
we explicitly require the X11 platform (and all its dependencies).

On the glx/dri/opengl side - I'm not claiming that the heuristics are
perfect, but if there's some issue (detection, dependencies and/or
other) in there we should fix it for everyone - just let me know
what's broken and I'll look into it. Please ?

>
>> Speaking of which - I cannot find any reports of people (mis)configuring
>> mesa, leading to lack of glamor/X11 acceleration.
>
> glamor failing to initialize in Xorg because Mesa was built without
> --with-egl-platforms=drm[,...] was a fairly common problem for radeonsi
> users until Marek made it basically impossible. Most of those reports
> might have been just on IRC though.
>
Ack. If anyone has a Xorg.0.log with a gbm-less libEGL handy that'll
be amazing. We could/should be able to make things clearer/easier to
track in such cases.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Michel Dänzer]

On 24/10/16 06:33 PM, Emil Velikov wrote:
> 
> IIRC glamor + glx isn't really an option.

That's true for Xorg, because Xorg is the thing which provides GLX in
the first place, so it cannot use GLX itself. (glamor on GLX works in
Xephyr though)


> So as we look the !glamor case (hmm does newer radeons have
> acceleration in those cases ?)

Not in X.


> Speaking of which - I cannot find any reports of people (mis)configuring
> mesa, leading to lack of glamor/X11 acceleration.

glamor failing to initialize in Xorg because Mesa was built without
--with-egl-platforms=drm[,...] was a fairly common problem for radeonsi
users until Marek made it basically impossible. Most of those reports
might have been just on IRC though.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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  wrote:
> On Wed, Oct 19, 2016 at 2:40 PM, Emil Velikov  
> wrote:
>> On 18 October 2016 at 23:00, Marek Olšák  wrote:
>>> From: Marek Olšák 
>>>
>>> ---
>>>  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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

On Wed, Oct 19, 2016 at 2:40 PM, Emil Velikov  wrote:
> On 18 October 2016 at 23:00, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> ---
>>  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


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

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.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Emil Velikov]

On 18 October 2016 at 23:00, Marek Olšák  wrote:
> From: Marek Olšák 
>
> ---
>  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.


1-3 as-is are
Reviewed-by: Emil Velikov 

v2 4/4 with the function dropped or a simplistic one alike above
Reviewed-by: Emil Velikov 

Thanks for untangling this !
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev