On 17 August 2018 at 19:57, Mathias Fröhlich <mathias.froehl...@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.veli...@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.froehl...@gmx.net>
>> Signed-off-by: Emil Velikov <emil.veli...@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.froehl...@web.de>
>
Agreed, squashed locally. Will give it a few days and push the series
~mid next week.
Thanks again, for the help.
Emil
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
