[Piglit] [PATCH v2] egl Add new test of EGL_EXT_platform_device

[Emil Velikov]

On 17 August 2018 at 19:57, Mathias Fröhlich <Mathias.Froehlich at gmx.net> wrote:
> Hi,
>
> Sorry for the late replay.
> I can also take a look at the mesa side but not before next week.
>
No worries. I'm grateful for any feedback - positive or negative,
immediate or not.

> Nevertheless, I still have one more comment inline below:
>
> On Tuesday, 14 August 2018 15:44:09 CEST Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Since the functionality is more or less identical to
>> EGL_MESA_platform_surfaceless, the test with a copy of it.
>>
>> Changes, as listed in the test itself, include:
>>  - s/MESA_platform_surfaceless/EXT_platform_device/g
>>  - entrypoint handling - eglQueryDeviceStringEXT, eglQueryDevicesEXT and eglGetPlatformDisplayEXT
>>  - custom GetDisplay, based on eglQueryDevicesEXT
>>  - couple of s/PIGLIT_SKIP/PIGLIT_FAIL/
>>
>> v2: Use eglQueryDeviceStringEXT to get a DRM backed device (Mathias)
>> Cc: Mathias Fröhlich <Mathias.Froehlich at gmx.net>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>> Thanks for the help Mathias. The updated patch should work as you
>> suggested.
>> ---
>>  tests/egl/spec/CMakeLists.txt                 |   1 +
>>  .../CMakeLists.no_api.txt                     |   7 +
>>  .../egl_ext_platform_device/CMakeLists.txt    |   1 +
>>  .../egl_ext_platform_device.c                 | 279 ++++++++++++++++++
>>  tests/opengl.py                               |   6 +
>>  5 files changed, 294 insertions(+)
>>  create mode 100644 tests/egl/spec/egl_ext_platform_device/CMakeLists.no_api.txt
>>  create mode 100644 tests/egl/spec/egl_ext_platform_device/CMakeLists.txt
>>  create mode 100644 tests/egl/spec/egl_ext_platform_device/egl_ext_platform_device.c
>>
>> diff --git a/tests/egl/spec/CMakeLists.txt b/tests/egl/spec/CMakeLists.txt
>> index 9324efcaf..f38a4f62b 100644
>> --- a/tests/egl/spec/CMakeLists.txt
>> +++ b/tests/egl/spec/CMakeLists.txt
>> @@ -3,6 +3,7 @@ add_subdirectory (egl_ext_client_extensions)
>>  add_subdirectory (egl_ext_device_query)
>>  add_subdirectory (egl_ext_device_enumeration)
>>  add_subdirectory (egl_ext_device_drm)
>> +add_subdirectory (egl_ext_platform_device)
>>  add_subdirectory (egl_ext_image_dma_buf_import_modifiers)
>>  add_subdirectory (egl_khr_create_context)
>>  add_subdirectory (egl_khr_get_all_proc_addresses)
>> diff --git a/tests/egl/spec/egl_ext_platform_device/CMakeLists.no_api.txt b/tests/egl/spec/egl_ext_platform_device/CMakeLists.no_api.txt
>> new file mode 100644
>> index 000000000..6c1bbd2a1
>> --- /dev/null
>> +++ b/tests/egl/spec/egl_ext_platform_device/CMakeLists.no_api.txt
>> @@ -0,0 +1,7 @@
>> +link_libraries(
>> +     piglitutil
>> +)
>> +
>> +piglit_add_executable(egl_ext_platform_device egl_ext_platform_device.c)
>> +
>> +# vim: ft=cmake:
>> diff --git a/tests/egl/spec/egl_ext_platform_device/CMakeLists.txt b/tests/egl/spec/egl_ext_platform_device/CMakeLists.txt
>> new file mode 100644
>> index 000000000..144a306f4
>> --- /dev/null
>> +++ b/tests/egl/spec/egl_ext_platform_device/CMakeLists.txt
>> @@ -0,0 +1 @@
>> +piglit_include_target_api()
>> diff --git a/tests/egl/spec/egl_ext_platform_device/egl_ext_platform_device.c b/tests/egl/spec/egl_ext_platform_device/egl_ext_platform_device.c
>> new file mode 100644
>> index 000000000..8d2db2381
>> --- /dev/null
>> +++ b/tests/egl/spec/egl_ext_platform_device/egl_ext_platform_device.c
>> @@ -0,0 +1,279 @@
>> +/*
>> + * Copyright 2018 Collabora, Ltd.
>> + *
>> + * Based on ext_mesa_platform_surfaceless.c which has
>> + * Copyright 2016 Google
>> + *
>> + * 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, sublicense,
>> + * 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.
>> + */
>> +
>> +
>> +/* Note that this test is a mere copy with the following changes:
>> + *  - s/MESA_platform_surfaceless/EXT_platform_device/g
>> + *  - entrypoint handling - eglQueryDeviceStringEXT, eglQueryDevicesEXT and eglGetPlatformDisplayEXT
>> + *  - custom GetDisplay, based on eglQueryDevicesEXT
>> + *  - couple of s/PIGLIT_SKIP/PIGLIT_FAIL/
>> + */
>> +
>> +#include "piglit-util.h"
>> +#include "piglit-util-egl.h"
>> +
>> +/* Extension function pointers.
>> + *
>> + * Use prefix 'pegl' (piglit egl) instead of 'egl' to avoid collisions with
>> + * prototypes in eglext.h. */
>> +EGLSurface (*peglCreatePlatformPixmapSurfaceEXT)(EGLDisplay display, EGLConfig config,
>> +         void *native_pixmap, const EGLint *attrib_list);
>> +EGLSurface (*peglCreatePlatformWindowSurfaceEXT)(EGLDisplay display, EGLConfig config,
>> +         void *native_window, const EGLint *attrib_list);
>> +
>> +const char *(*peglQueryDeviceStringEXT)(EGLDeviceEXT device, EGLint name);
>> +EGLBoolean (*peglQueryDevicesEXT)(EGLint max_devices, EGLDeviceEXT *devices,
>> +         EGLint *num_devices);
>> +EGLDisplay (*peglGetPlatformDisplayEXT)(EGLenum platform, void *native_display,
>> +         const EGLint *attrib_list);
>> +
>> +static void
>> +init_egl_extension_funcs(void)
>> +{
>> +     peglCreatePlatformPixmapSurfaceEXT = (void*)
>> +             eglGetProcAddress("eglCreatePlatformPixmapSurfaceEXT");
>> +     peglCreatePlatformWindowSurfaceEXT = (void*)
>> +             eglGetProcAddress("eglCreatePlatformWindowSurfaceEXT");
>> +
>> +     peglQueryDeviceStringEXT = (void *)eglGetProcAddress("eglQueryDeviceStringEXT");
>> +     peglQueryDevicesEXT = (void *)eglGetProcAddress("eglQueryDevicesEXT");
>> +     peglGetPlatformDisplayEXT = (void *)eglGetProcAddress("eglGetPlatformDisplayEXT");
>> +}
>> +
>> +static EGLDisplay *
>> +get_device_display(void)
>> +{
>> +#define NDEVS 1024
>> +     EGLDeviceEXT devices[NDEVS];
>> +     EGLint i, num_devices;
>> +     const char *devstring;
>> +
>> +     if (!peglQueryDevicesEXT(NDEVS, devices, &num_devices)) {
>> +             printf("Failed to get egl device\n");
>> +             piglit_report_result(PIGLIT_FAIL);
>> +     }
>> +     /* Use a DRM device, as the software one has some issues. */
>> +     for (i = 0; i < num_devices; i++) {
>> +             devstring = peglQueryDeviceStringEXT(devices[i], EGL_EXTENSIONS);
>> +             if (piglit_is_extension_in_string(devstring,
>> +                                     "EGL_EXT_device_drm")) {
>> +                     return peglGetPlatformDisplayEXT(EGL_PLATFORM_DEVICE_EXT,
>> +                                                     devices[i], NULL);
>> +             }
>> +     }
>> +     printf("Failed to get a drm backed, egl device\n");
>> +     piglit_report_result(PIGLIT_FAIL);
>
>
> I would prefer to get a PIGLIT_SKIP if there is no device offering
> EGL_EXT_device_drm is present.
> I think its legal for a driver not to offer any device with this extension.
>
>
> With that changed:
> Reviewed-by: Mathias Fröhlich <mathias.froehlich at web.de>
>
Agreed, squashed locally. Will give it a few days and push the series
~mid next week.

Thanks again, for the help.
Emil