[Mesa-dev] [PATCH v2 1/9] egl: add a treatment of tizen platform on egl display

[Emil Velikov]

On 17 September 2017 at 19:00, Gwan-gyeong Mun <elongbug at gmail.com> wrote:
> It adds a _EGL_PLATFORM_TIZEN enum value to _EGLPlatformType for tizen platform.
>
> It adds a detecting routine of tizen platform to _eglNativePlatformDetectNativeDisplay()
> and _eglGetNativePlatform().
>   - As tizen platform internally distinguishes native displays of drm/gbm and wayland
>     client, when EGL_PLATFORM_WAYLAND_EXT or EGL_PLATFORM_GBM_MESA come from
>     eglGetPlatformDisplayEXT(), it have call _eglGetTizenDisplay().
Hmmm are you saying that Tizen is only is a wrapper? If one already
uses the Wayland/GBM platform why do we need the separate backend?

I think this is a good example where a spec with some information will
be useful.

>   - If tizen platform is enabled at the configuration, _eglGetNativePlatform()
>     always detects _EGL_PLATFORM_TIZEN as a detected_platform.
>
> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
> ---
>  src/egl/main/eglapi.c     | 12 ++++++++++++
>  src/egl/main/egldisplay.c | 30 ++++++++++++++++++++++++++++++
>  src/egl/main/egldisplay.h |  7 +++++++
>  3 files changed, 49 insertions(+)
>
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index e3f10fcbe2..12e867ca72 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -404,6 +404,18 @@ _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
>        dpy = _eglGetSurfacelessDisplay(native_display, attrib_list);
>        break;
>  #endif
> +#ifdef HAVE_TIZEN_PLATFORM
> +#ifndef HAVE_DRM_PLATFORM
> +    case EGL_PLATFORM_GBM_MESA:
> +       dpy = _eglGetTizenDisplay(native_display, attrib_list);
> +       break;
> +#endif /* not HAVE_DRM_PLATFORM */
> +#ifndef HAVE_WAYLAND_PLATFORM
> +    case EGL_PLATFORM_WAYLAND_EXT:
> +       dpy = _eglGetTizenDisplay(native_display, attrib_list);
> +       break;
> +#endif /* not HAVE_WAYLAND_PLATFORM */
> +#endif /* HAVE_TIZEN_PLATFORM */

Please add a comment about the ifndef blocks.

>     default:
>        RETURN_EGL_ERROR(NULL, EGL_BAD_PARAMETER, NULL);
>     }
> diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> index 690728d2f7..b55d243015 100644
> --- a/src/egl/main/egldisplay.c
> +++ b/src/egl/main/egldisplay.c
> @@ -70,6 +70,7 @@ static const struct {
>     { _EGL_PLATFORM_ANDROID, "android" },
>     { _EGL_PLATFORM_HAIKU, "haiku" },
>     { _EGL_PLATFORM_SURFACELESS, "surfaceless" },
> +   { _EGL_PLATFORM_TIZEN, "tizen" },
>  };
>
>
> @@ -115,6 +116,13 @@ _eglNativePlatformDetectNativeDisplay(void *nativeDisplay)
>
>        (void) first_pointer; /* silence unused var warning */
>
> +#ifdef HAVE_TIZEN_PLATFORM
> +      /* In Tizen Platform, _EGL_PLATFORM_TIZEN treats together DRM(gbm) platform
> +       * and wayland egl platform.
Seems to be one of those things that did not translate well. Try
another approach - I'm struggling to understand what you mean here.

Regardless of Wayland/GBM a proper detection is what you want here.

> +       */
> +      return _EGL_PLATFORM_TIZEN;
> +#endif
> +
>  #ifdef HAVE_WAYLAND_PLATFORM
>        /* wl_display is a wl_proxy, which is a wl_object.
>         * wl_object's first element points to the interfacetype. */
> @@ -157,6 +165,13 @@ _eglGetNativePlatform(void *nativeDisplay)
>        detected_platform = _eglGetNativePlatformFromEnv();
>        detection_method = "environment overwrite";
>
> +#ifdef HAVE_TIZEN_PLATFORM
> +      if (detected_platform != _EGL_PLATFORM_TIZEN) {
> +         detected_platform = _EGL_PLATFORM_TIZEN;
> +         detection_method = "build-time configuration";
> +      }
> +#endif
> +
Not needed - the build-time bit is further down.
Also it seems wrong - order must be: env override -> autodetect ->
build-time default

-Emil