[PATCH xserver] glamor_egl: request GL2.1 when requesting Desktop GL context

[Emil Velikov]

On 21 August 2018 at 17:01, Icenowy Zheng <icenowy at aosc.io> wrote:
> Some devices cannot support OpenGL 2.1, which is the minimum desktop GL
> version required by glamor. However, they may support OpenGL ES 2.0,
> which is the GLES version required by glamor. Usually in this situation
> the desktop GL version supported is 2.0 or 1.4.
>
> Currently, as no requirements are passed when creating desktop GL
> context, a OpenGL 1.4/2.0 context will be created, and glamor will
> arguing that the context is not suitable, although the GPU supports a
> suitable GLES context.
>
> Add version number 2.1 requirement when requesting non-core desktop GL
> context (core context has at least 3.1), so it will fall back to create
> GLES contexts when the version number requirement is not met.
>
I don't know glamor enough to say if OpenGL 2.1 or another version
should be required.
Small mildly related note below.

> Tested on a Intel 945GMS integrated GPU, which supports GL 1.4 and GLES
> 2.0. Before applying this, it will fail to launch X server when no
> configuration is present because of glamor initialization failure, after
> applying glamor will start with GLES.
>
> Signed-off-by: Icenowy Zheng <icenowy at aosc.io>
> ---
>  glamor/glamor.h     | 4 ++++
>  glamor/glamor_egl.c | 4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/glamor/glamor.h b/glamor/glamor.h
> index 09e9c895c..abee6a3e8 100644
> --- a/glamor/glamor.h
> +++ b/glamor/glamor.h
> @@ -75,6 +75,10 @@ typedef Bool (*GetDrawableModifiersFuncPtr) (DrawablePtr draw,
>  #define GLAMOR_GL_CORE_VER_MAJOR 3
>  #define GLAMOR_GL_CORE_VER_MINOR 1
>
> +/* We need OpenGL 2.1 to work at least */
> +#define GLAMOR_GL_VER_MAJOR 2
> +#define GLAMOR_GL_VER_MINOR 1
> +
>  /* @glamor_init: Initialize glamor internal data structure.
>   *
>   * @screen: Current screen pointer.
> diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
> index b33d8ef15..8da8d2731 100644
> --- a/glamor/glamor_egl.c
> +++ b/glamor/glamor_egl.c
> @@ -946,6 +946,10 @@ glamor_egl_init(ScrnInfoPtr scrn, int fd)
>              EGL_NONE
>          };
>          static const EGLint config_attribs[] = {
> +            EGL_CONTEXT_MAJOR_VERSION_KHR,
> +           GLAMOR_GL_VER_MAJOR,
> +           EGL_CONTEXT_MINOR_VERSION_KHR,
The EGL attributes are part of EGL_KHR_create_context, which we do not
check currently.
Since EGL_CONTEXT_MAJOR_VERSION_KHR is effectively an alias for
EGL_CONTEXT_CLIENT_VERSION one could use the latter and GL_VERSION of
the created context.

it's technically correct, although it feels like an overkill.
Worth checking what others think on the topic.

HTH
Emil