[Mesa-dev] [PATCH 1/2] egl/dri2: implement platform_null (v2).

Chad Versace chad.versace at intel.com
Fri Apr 3 13:35:27 PDT 2015


Time to revive this patch!

Why?
  - I don't like large patchsets living in Chrome OS for too long.
  - Frank submitted Waffle patches to support this, and I hesitate to
    add Waffle support unless the platform is upstream.

Of course, the patch no longer applies to master. So I rebased and
pushed it to a personal branch:

    git fetch git://github.com/chadversary/mesa refs/heads/cooking/hshi/egl-platform-null && git checkout FETCH_HEAD
    https://github.com/chadversary/mesa/tree/cooking/hshi/egl-platform-null

On Tue 17 Feb 2015, Haixia Shi wrote:
>The NULL platform is for off-screen rendering only. Render node support is
>required.

Naming it the "NULL" platform seems very odd. It actually *does* stuff.
Usually things named "null" do nothing.

Also, this platform is not unique in its NULL requirement. That is,
there do exist other EGL platforms in which eglGetDisplay accepts only
NULL, such as Android. I strongly suspect this is also the case for
other lesser known operating systems.

But there isn't an obviously better name. Me and Jordan chatted about
this for a long time and came to no conclusion.

Usually, an EGL platform is named after (1) the operating system, (2)
the underlying window system, or (3) the real type of
EGLNativeDisplayType. None of those precedents help here. However, this
platform does have a single, unique property that distinguishes it from
all other EGL platforms: ___this is the only platform that has no
support for EGLSurface___. Perhaps we should name the platform after
that property?

Perhaps EGL_MESA_platform_surfaceless and platform_surfaceless.c?

>Only consider the render nodes. Do not use normal nodes as they require
>auth hooks.

I agree with that decision. The platform should not fallback to
/dev/dri/card*, at least not in this initial patch. That adds too much
complication.

I have comments below on how the rendernode gets selected.

>Signed-off-by: Haixia Shi <hshi at chromium.org>
>---
> src/egl/drivers/dri2/Makefile.am     |   5 ++
> src/egl/drivers/dri2/egl_dri2.c      |  13 ++-
> src/egl/drivers/dri2/egl_dri2.h      |   3 +
> src/egl/drivers/dri2/platform_null.c | 169 +++++++++++++++++++++++++++++++++++
> 4 files changed, 187 insertions(+), 3 deletions(-)
> create mode 100644 src/egl/drivers/dri2/platform_null.c
>
>diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
>index 86e5f24..6ed137e 100644
>--- a/src/egl/drivers/dri2/egl_dri2.c
>+++ b/src/egl/drivers/dri2/egl_dri2.c

>@@ -632,6 +632,13 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp)
>       return EGL_FALSE;
>
>    switch (disp->Platform) {
>+#ifdef HAVE_NULL_PLATFORM
>+   case _EGL_PLATFORM_NULL:
>+      if (disp->Options.TestOnly)
>+         return EGL_TRUE;
>+      return dri2_initialize_null(drv, disp);
>+#endif

The platform has a major deficiency in this hunk, but I don't think it
needs fixing in this initial patch. Due to the way
eglGetDisplay(EGLNativeDisplay dpy)
auto-detects the real type of dpy, it's impossible to build Mesa
with platform_null and platform_x11 enabled and actually have both
usable. eglGetDisplay will work for only one, and the one that works
will be the first that occurs in the platform list given to
--with-egl-platforms=... .  In other words,

    --with-egl-platforms=x11,null  => eglGetDisplay(NULL) opens the default X11 display
    --with-egl-platforms=null,x11  => eglGetDisplay(NULL) opens a render node

Again, I don't think you need to fix this in the initial patch. The
proper solution is to implement a platform extension, like
EGL_MESA_platform_gbm [1], which can be done in a follow-up patch.
Without a platform extension, distro packagers and most Mesa developers
will not be able to ever test this platform.

[1] https://www.khronos.org/registry/egl/extensions/MESA/EGL_MESA_platform_gbm.txt

>diff --git a/src/egl/drivers/dri2/platform_null.c b/src/egl/drivers/dri2/platform_null.c
>new file mode 100644
>index 0000000..55ceab6
>--- /dev/null
>+++ b/src/egl/drivers/dri2/platform_null.c
>@@ -0,0 +1,169 @@


>+static __DRIbuffer *
>+null_get_buffers_with_format(__DRIdrawable * driDrawable,
>+                             int *width, int *height,
>+                             unsigned int *attachments, int count,
>+                             int *out_count, void *loaderPrivate)
>+{
>+   struct dri2_egl_surface *dri2_surf = loaderPrivate;
>+   struct dri2_egl_display *dri2_dpy =
>+      dri2_egl_display(dri2_surf->base.Resource.Display);

dri2_dpy is unused.


>+
>+   dri2_surf->buffer_count = 1;
>+   if (width)
>+      *width = dri2_surf->base.Width;
>+   if (height)
>+      *height = dri2_surf->base.Height;
>+   *out_count = dri2_surf->buffer_count;;
>+   return dri2_surf->buffers;
>+}
>+
>+#define DRM_RENDER_DEV_NAME  "%s/renderD%d"
>+
>+EGLBoolean
>+dri2_initialize_null(_EGLDriver *drv, _EGLDisplay *disp)
>+{
>+   struct dri2_egl_display *dri2_dpy;
>+   const char* err;
>+   int i, render_node;

render_node is unused.


>+   int driver_loaded = 0;
>+
>+   loader_set_logger(_eglLog);
>+
>+   dri2_dpy = calloc(1, sizeof *dri2_dpy);
>+   if (!dri2_dpy)
>+      return _eglError(EGL_BAD_ALLOC, "eglInitialize");
>+
>+   disp->DriverData = (void *) dri2_dpy;
>+
>+   const int limit = 64;
>+   const int base = 128;
>+   for (i = 0; i < limit; ++i) {
>+      char *card_path;
>+      if (asprintf(&card_path, DRM_RENDER_DEV_NAME, DRM_DIR_NAME, base + i) < 0)
>+         continue;

Manually looping over render node names feels slightly wrong. You could
use udev instead, but that might be overkill. Anyways... I'm unable to
suggest a conclusively better method, so this hunk is ok with me.


>+   for (i = 0; dri2_dpy->driver_configs[i]; i++) {
>+      EGLint attr_list[1];
>+      attr_list[0] = EGL_NONE;
>+      dri2_add_config(disp, dri2_dpy->driver_configs[i],
>+                      i + 1, EGL_WINDOW_BIT, attr_list, NULL);

The platform doesn't support eglCreateWindowSurface, so why do you set
EGL_WINDOW_BIT here?  Is this to work around some limitiation of
dri2_add_config?


More information about the mesa-dev mailing list