[Mesa-dev] [PATCH v2 1/8] egl: add base EGL_EXT_device_base implementation

Mathias Fröhlich Mathias.Froehlich at gmx.net
Thu Sep 20 14:11:30 UTC 2018


Hi Emil,

On Tuesday, 4 September 2018 20:32:58 CEST Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> Introduce the API for device query and enumeration. Those at the moment
> produce nothing useful since zero devices are actually available.
> 
> That contradicts with the spec, so the extension isn't advertised just
> yet.
> 
> With later commits we'll add support for software (always) and hardware
> devices. Each one exposing the respective extension string.
> 
> v2:
>  - fold API boilerplate into this patch
>  - move _eglAddDevice, _eglDeviceSupports, _eglRefreshDeviceList to this
> patch (Eric, Mathias)
>  - make _eglFiniDevice the one called last

Thanks for the updated series.

Nevertheless, patches #1, #2, #3, #8 have comments.
Especialy there are lots of asserts that either dont compile or assert on else working code.
You should double check what they are supposed to do.

So you will find also here some comments inline:

> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
>  src/egl/Makefile.sources     |   2 +
>  src/egl/main/eglapi.c        |  64 +++++++++++++
>  src/egl/main/egldevice.c     | 179 +++++++++++++++++++++++++++++++++++
>  src/egl/main/egldevice.h     |  83 ++++++++++++++++
>  src/egl/main/egldisplay.h    |   1 +
>  src/egl/main/eglentrypoint.h |   4 +
>  src/egl/main/eglglobals.c    |   8 +-
>  src/egl/main/eglglobals.h    |   2 +
>  src/egl/main/egltypedefs.h   |   2 +
>  src/egl/meson.build          |   2 +
>  10 files changed, 344 insertions(+), 3 deletions(-)
>  create mode 100644 src/egl/main/egldevice.c
>  create mode 100644 src/egl/main/egldevice.h
> 
> diff --git a/src/egl/Makefile.sources b/src/egl/Makefile.sources
> index 82f13ad3cbd..0cc5f1bbfef 100644
> --- a/src/egl/Makefile.sources
> +++ b/src/egl/Makefile.sources
> @@ -10,6 +10,8 @@ LIBEGL_C_FILES := \
>  	main/eglcurrent.c \
>  	main/eglcurrent.h \
>  	main/egldefines.h \
> +	main/egldevice.c \
> +	main/egldevice.h \
>  	main/egldisplay.c \
>  	main/egldisplay.h \
>  	main/egldriver.c \
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index c8c6a50f6ad..6df5e841463 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -95,6 +95,7 @@
>  #include "egldisplay.h"
>  #include "egltypedefs.h"
>  #include "eglcurrent.h"
> +#include "egldevice.h"
>  #include "egldriver.h"
>  #include "eglsurface.h"
>  #include "eglconfig.h"
> @@ -2575,6 +2576,69 @@ eglSetBlobCacheFuncsANDROID(EGLDisplay *dpy, EGLSetBlobFuncANDROID set,
>     _eglUnlockDisplay(disp);
>  }
>  
> +static EGLBoolean EGLAPIENTRY
> +eglQueryDeviceAttribEXT(EGLDeviceEXT device,
> +                        EGLint attribute,
> +                        EGLAttrib *value)
> +{
> +   _EGLDevice *dev = _eglLookupDevice(device);
> +   EGLBoolean ret;
> +
> +   _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_FALSE);
> +   if (!dev)
> +      RETURN_EGL_ERROR(NULL, EGL_BAD_DEVICE_EXT, EGL_FALSE);
> +
> +   ret = _eglQueryDeviceAttribEXT(dev, attribute, value);
> +   RETURN_EGL_EVAL(NULL, ret);
> +}
> +
> +static const char * EGLAPIENTRY
> +eglQueryDeviceStringEXT(EGLDeviceEXT device,
> +                        EGLint name)
> +{
> +   _EGLDevice *dev = _eglLookupDevice(device);
> +
> +   _EGL_FUNC_START(NULL, EGL_NONE, NULL, NULL);
> +   if (!dev)
> +      RETURN_EGL_ERROR(NULL, EGL_BAD_DEVICE_EXT, NULL);
> +
> +   RETURN_EGL_EVAL(NULL, _eglQueryDeviceStringEXT(dev, name));
> +}
> +
> +static EGLBoolean EGLAPIENTRY
> +eglQueryDevicesEXT(EGLint max_devices,
> +                   EGLDeviceEXT *devices,
> +                   EGLint *num_devices)
> +{
> +   EGLBoolean ret;
> +
> +   _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_FALSE);
> +   ret = _eglQueryDevicesEXT(max_devices, (_EGLDevice **) devices,
> +                             num_devices);
> +   RETURN_EGL_EVAL(NULL, ret);
> +}
> +
> +static EGLBoolean EGLAPIENTRY
> +eglQueryDisplayAttribEXT(EGLDisplay dpy,
> +                         EGLint attribute,
> +                         EGLAttrib *value)
> +{
> +   _EGLDisplay *disp = _eglLockDisplay(dpy);
> +   _EGLDriver *drv;
> +
> +   _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_FALSE);
> +   _EGL_CHECK_DISPLAY(disp, EGL_FALSE, drv);
> +
> +   switch (attribute) {
> +   case EGL_DEVICE_EXT:
> +      *value = (EGLAttrib) disp->Device;
> +      break;
> +   default:
> +      RETURN_EGL_ERROR(disp, EGL_BAD_ATTRIBUTE, EGL_FALSE);
> +   }
> +   RETURN_EGL_SUCCESS(disp, EGL_TRUE);
> +}
> +
>  __eglMustCastToProperFunctionPointerType EGLAPIENTRY
>  eglGetProcAddress(const char *procname)
>  {
> diff --git a/src/egl/main/egldevice.c b/src/egl/main/egldevice.c
> new file mode 100644
> index 00000000000..bbc9f2060d1
> --- /dev/null
> +++ b/src/egl/main/egldevice.c
> @@ -0,0 +1,179 @@
> +/**************************************************************************
> + *
> + * Copyright 2015, 2018 Collabora
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + **************************************************************************/
> +
> +#include "util/macros.h"
> +
> +#include "eglcurrent.h"
> +#include "egldevice.h"
> +#include "eglglobals.h"
> +#include "egltypedefs.h"
> +
> +
> +struct _egl_device {
> +   _EGLDevice *Next;
> +
> +   const char *extensions;
> +};
> +
> +void
> +_eglFiniDevice(void)
> +{
> +   _EGLDevice *dev_list, *dev;
> +
> +   /* atexit function is called with global mutex locked */
> +
> +   dev_list = _eglGlobal.DeviceList;
> +   while (dev_list) {
> +      /* pop list head */
> +      dev = dev_list;
> +      dev_list = dev_list->Next;
> +
> +      free(dev);
> +   }
> +
> +   _eglGlobal.DeviceList = NULL;
> +}
> +
> +EGLBoolean
> +_eglCheckDeviceHandle(EGLDeviceEXT device)
> +{
> +   _EGLDevice *cur;
> +
> +   mtx_lock(_eglGlobal.Mutex);
> +   cur = _eglGlobal.DeviceList;
> +   while (cur) {
> +      if (cur == (_EGLDevice *) device)
> +         break;
> +      cur = cur->Next;
> +   }
> +   mtx_unlock(_eglGlobal.Mutex);
> +   return (cur != NULL);
> +}
> +
> +/* Adds a device in DeviceList, if needed for the given fd.
> + *
> + * If a software device, the fd is ignored.
> + */
> +_EGLDevice *
> +_eglAddDevice(int fd, bool software)
> +{
> +   _EGLDevice *dev;
> +
> +   mtx_lock(_eglGlobal.Mutex);
> +
> +   dev = NULL;
> +
> +out:
> +   mtx_unlock(_eglGlobal.Mutex);
> +   return dev;
> +}
> +
> +EGLBoolean
> +_eglDeviceSupports(_EGLDevice *dev, _EGLDeviceExtension ext)
> +{
> +   switch (ext) {
> +   default:
> +      assert(0);
> +      return EGL_FALSE;
> +   };
> +}
> +
> +EGLBoolean
> +_eglQueryDeviceAttribEXT(_EGLDevice *dev, EGLint attribute,
> +                         EGLAttrib *value)
> +{
> +   switch (attribute) {
> +   default:
> +      _eglError(EGL_BAD_ATTRIBUTE, "eglQueryDeviceStringEXT");
> +      return EGL_FALSE;
> +   }
> +}
> +
> +const char *
> +_eglQueryDeviceStringEXT(_EGLDevice *dev, EGLint name)
> +{
> +   switch (name) {
> +   case EGL_EXTENSIONS:
> +      return dev->extensions;
> +   default:
> +      _eglError(EGL_BAD_PARAMETER, "eglQueryDeviceStringEXT");
> +      return NULL;
> +   };
> +}
> +
> +/* Do a fresh lookup for devices.
> + *
> + * Walks through the DeviceList, discarding no longer available ones
> + * and adding new ones as applicable.
> + *
> + * Must be called with the global lock held.
> + */
> +static int
> +_eglRefreshDeviceList(void)
> +{
> +   _EGLDevice *dev;
> +   int count = 0;
> +
> +   dev = _eglGlobal.DeviceList;

That one gives a compile warning in release compiles.

Even in the final version of the file past the whole patch series.
The dev argument is only used in asserts finally.

best

Mathias


> +
> +   return count;
> +}
> +
> +EGLBoolean
> +_eglQueryDevicesEXT(EGLint max_devices,
> +                    _EGLDevice **devices,
> +                    EGLint *num_devices)
> +{
> +   _EGLDevice *dev, *devs;
> +   int i = 0, num_devs;
> +
> +   if ((devices && max_devices <= 0) || !num_devices)
> +      return _eglError(EGL_BAD_PARAMETER, "eglQueryDevicesEXT");
> +
> +   mtx_lock(_eglGlobal.Mutex);
> +
> +   num_devs = _eglRefreshDeviceList();
> +   devs = _eglGlobal.DeviceList;
> +
> +   /* bail early if we only care about the count */
> +   if (!devices) {
> +      *num_devices = num_devs;
> +      goto out;
> +   }
> +
> +   *num_devices = MIN2(num_devs, max_devices);
> +
> +   for (i = 0, dev = devs; i < *num_devices; i++) {
> +      devices[i] = dev;
> +      dev = dev->Next;
> +   }
> +
> +out:
> +   mtx_unlock(_eglGlobal.Mutex);
> +
> +   return EGL_TRUE;
> +}
> diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h
> new file mode 100644
> index 00000000000..72d250393f4
> --- /dev/null
> +++ b/src/egl/main/egldevice.h
> @@ -0,0 +1,83 @@
> +/**************************************************************************
> + *
> + * Copyright 2015, 2018 Collabora
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + **************************************************************************/
> +
> +
> +#ifndef EGLDEVICE_INCLUDED
> +#define EGLDEVICE_INCLUDED
> +
> +
> +#include <stdbool.h>
> +#include "egltypedefs.h"
> +
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +void
> +_eglFiniDevice(void);
> +
> +EGLBoolean
> +_eglCheckDeviceHandle(EGLDeviceEXT device);
> +
> +static inline _EGLDevice *
> +_eglLookupDevice(EGLDeviceEXT device)
> +{
> +   _EGLDevice *dev = (_EGLDevice *) device;
> +   if (!_eglCheckDeviceHandle(device))
> +      dev = NULL;
> +   return dev;
> +}
> +
> +_EGLDevice *
> +_eglAddDevice(int fd, bool software);
> +
> +enum _egl_device_extension {
> +   EGL_FOOBAR,
> +};
> +
> +typedef enum _egl_device_extension _EGLDeviceExtension;
> +
> +EGLBoolean
> +_eglDeviceSupports(_EGLDevice *dev, _EGLDeviceExtension ext);
> +
> +EGLBoolean
> +_eglQueryDeviceAttribEXT(_EGLDevice *dev, EGLint attribute,
> +                         EGLAttrib *value);
> +
> +const char *
> +_eglQueryDeviceStringEXT(_EGLDevice *dev, EGLint name);
> +
> +EGLBoolean
> +_eglQueryDevicesEXT(EGLint max_devices, _EGLDevice **devices,
> +                    EGLint *num_devices);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* EGLDEVICE_INCLUDED */
> diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
> index 50176bde887..fae90520368 100644
> --- a/src/egl/main/egldisplay.h
> +++ b/src/egl/main/egldisplay.h
> @@ -159,6 +159,7 @@ struct _egl_display
>     _EGLPlatformType Platform; /**< The type of the platform display */
>     void *PlatformDisplay;     /**< A pointer to the platform display */
>  
> +   _EGLDevice *Device;        /**< Device backing the display */
>     _EGLDriver *Driver;        /**< Matched driver of the display */
>     EGLBoolean Initialized;    /**< True if the display is initialized */
>  
> diff --git a/src/egl/main/eglentrypoint.h b/src/egl/main/eglentrypoint.h
> index 06749f6353a..69a6c1bf4c7 100644
> --- a/src/egl/main/eglentrypoint.h
> +++ b/src/egl/main/eglentrypoint.h
> @@ -56,6 +56,10 @@ EGL_ENTRYPOINT(eglPostSubBufferNV)
>  EGL_ENTRYPOINT(eglQueryAPI)
>  EGL_ENTRYPOINT(eglQueryContext)
>  EGL_ENTRYPOINT(eglQueryDebugKHR)
> +EGL_ENTRYPOINT(eglQueryDeviceAttribEXT)
> +EGL_ENTRYPOINT(eglQueryDeviceStringEXT)
> +EGL_ENTRYPOINT(eglQueryDevicesEXT)
> +EGL_ENTRYPOINT(eglQueryDisplayAttribEXT)
>  EGL_ENTRYPOINT(eglQueryDmaBufFormatsEXT)
>  EGL_ENTRYPOINT(eglQueryDmaBufModifiersEXT)
>  EGL_ENTRYPOINT(eglQueryString)
> diff --git a/src/egl/main/eglglobals.c b/src/egl/main/eglglobals.c
> index c506bd6a9db..ac8bb3f328a 100644
> --- a/src/egl/main/eglglobals.c
> +++ b/src/egl/main/eglglobals.c
> @@ -35,6 +35,7 @@
>  #include "c11/threads.h"
>  
>  #include "eglglobals.h"
> +#include "egldevice.h"
>  #include "egldisplay.h"
>  #include "egldriver.h"
>  #include "egllog.h"
> @@ -51,11 +52,12 @@ struct _egl_global _eglGlobal =
>  {
>     .Mutex = &_eglGlobalMutex,
>     .DisplayList = NULL,
> -   .NumAtExitCalls = 2,
> +   .NumAtExitCalls = 3,
>     .AtExitCalls = {
>        /* default AtExitCalls, called in reverse order */
> -      _eglUnloadDrivers, /* always called last */
> -      _eglFiniDisplay
> +      _eglFiniDevice, /* always called last */
> +      _eglUnloadDrivers,
> +      _eglFiniDisplay,
>     },
>  
>     .ClientOnlyExtensionString =
> diff --git a/src/egl/main/eglglobals.h b/src/egl/main/eglglobals.h
> index 6655ccab65c..63bea4ebc38 100644
> --- a/src/egl/main/eglglobals.h
> +++ b/src/egl/main/eglglobals.h
> @@ -54,6 +54,8 @@ struct _egl_global
>     /* the list of all displays */
>     _EGLDisplay *DisplayList;
>  
> +   _EGLDevice *DeviceList;
> +
>     EGLint NumAtExitCalls;
>     void (*AtExitCalls[10])(void);
>  
> diff --git a/src/egl/main/egltypedefs.h b/src/egl/main/egltypedefs.h
> index 19524a16c42..642f473c4eb 100644
> --- a/src/egl/main/egltypedefs.h
> +++ b/src/egl/main/egltypedefs.h
> @@ -46,6 +46,8 @@ typedef struct _egl_config _EGLConfig;
>  
>  typedef struct _egl_context _EGLContext;
>  
> +typedef struct _egl_device _EGLDevice;
> +
>  typedef struct _egl_display _EGLDisplay;
>  
>  typedef struct _egl_driver _EGLDriver;
> diff --git a/src/egl/meson.build b/src/egl/meson.build
> index e11e589b945..80dbcae0fd3 100644
> --- a/src/egl/meson.build
> +++ b/src/egl/meson.build
> @@ -38,6 +38,8 @@ files_egl = files(
>    'main/eglcurrent.c',
>    'main/eglcurrent.h',
>    'main/egldefines.h',
> +  'main/egldevice.c',
> +  'main/egldevice.h',
>    'main/egldisplay.c',
>    'main/egldisplay.h',
>    'main/egldriver.c',
> 






More information about the mesa-dev mailing list