tests/egl/egl-context-priority.c: Use piglit_egl_get_default_display

Message ID 20171124163155.5449-1-limon.anibal@gmail.com
State New
Headers show
Series
  • tests/egl/egl-context-priority.c: Use piglit_egl_get_default_display
Related show

Commit Message

Aníbal Limón Nov. 24, 2017, 4:31 p.m.
From: Aníbal Limón <anibal.limon@linaro.org>

Some EGL implementations do not actually ship all Khronos-extensions.
As it turns out, the Mali 450 driver does not include eglGetPlatformDisplay
symbol so there is not grauntee to exists use piglit_egl_get_default_display
wrapper instead. See rev 45095dc08b.

Signed-off-by: Aníbal Limón <anibal.limon@linaro.org>
Signed-off-by: Daniel Díaz <daniel.diaz@linaro.org>
---
 tests/egl/egl-context-priority.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Tapani Pälli Nov. 27, 2017, 7:27 a.m. | #1
Right, this works because Mali 450 driver does not support 
EGL_MESA_platform_surfaceless, LGTM

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

On 11/24/2017 06:31 PM, Aníbal Limón wrote:
> From: Aníbal Limón <anibal.limon@linaro.org>
> 
> Some EGL implementations do not actually ship all Khronos-extensions.
> As it turns out, the Mali 450 driver does not include eglGetPlatformDisplay
> symbol so there is not grauntee to exists use piglit_egl_get_default_display

This needs to be rewritten with proper English, not sure how. Maybe simply:

"Mali 450 driver does not support eglGetPlatformDisplay, use 
piglit_egl_get_default_display wrapper instead."


> wrapper instead. See rev 45095dc08b.
> 
> Signed-off-by: Aníbal Limón <anibal.limon@linaro.org>
> Signed-off-by: Daniel Díaz <daniel.diaz@linaro.org>
> ---
>   tests/egl/egl-context-priority.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/egl/egl-context-priority.c b/tests/egl/egl-context-priority.c
> index 7f26fc614..9590ccae3 100644
> --- a/tests/egl/egl-context-priority.c
> +++ b/tests/egl/egl-context-priority.c
> @@ -99,8 +99,7 @@ piglit_init(int argc, char **argv)
>   	if (!strstr(exts, "EGL_MESA_platform_surfaceless"))
>   		piglit_report_result(PIGLIT_SKIP);
>   
> -	dpy = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA,
> -				    EGL_DEFAULT_DISPLAY, NULL);
> +	dpy = piglit_egl_get_default_display(EGL_PLATFORM_SURFACELESS_MESA);
>   
>   	ok = eglInitialize(dpy, &major, &minor);
>   	if (!ok) {
>
Emil Velikov Nov. 27, 2017, 1:44 p.m. | #2
On 27 November 2017 at 07:27, Tapani Pälli <tapani.palli@intel.com> wrote:
> Right, this works because Mali 450 driver does not support
> EGL_MESA_platform_surfaceless, LGTM
>
Even if the driver did support the extension a static entry-point to
link against is not guaranteed to be available.
The EGL spec says that portable applications should use
dlsym/eglGetProcAddress... which the helper does ;-)

I've looked at updating Mesa to hide these symbols but it's a bit tricky.

> Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
>
> On 11/24/2017 06:31 PM, Aníbal Limón wrote:
>>
>> From: Aníbal Limón <anibal.limon@linaro.org>
>>
>> Some EGL implementations do not actually ship all Khronos-extensions.
>> As it turns out, the Mali 450 driver does not include
>> eglGetPlatformDisplay
>> symbol so there is not grauntee to exists use
>> piglit_egl_get_default_display
>
>
> This needs to be rewritten with proper English, not sure how. Maybe simply:
>
> "Mali 450 driver does not support eglGetPlatformDisplay, use
> piglit_egl_get_default_display wrapper instead."
>
>
Looks good, one could also reuse the text from 45095dc08b.
In either case, patch is
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
Tapani Pälli Nov. 28, 2017, 7:45 a.m. | #3
I've changed commit msg to reuse the text from 45095dc08b and pushed the 
patch in, thanks!

On 11/27/2017 03:44 PM, Emil Velikov wrote:
> On 27 November 2017 at 07:27, Tapani Pälli <tapani.palli@intel.com> wrote:
>> Right, this works because Mali 450 driver does not support
>> EGL_MESA_platform_surfaceless, LGTM
>>
> Even if the driver did support the extension a static entry-point to
> link against is not guaranteed to be available.
> The EGL spec says that portable applications should use
> dlsym/eglGetProcAddress... which the helper does ;-)
> 
> I've looked at updating Mesa to hide these symbols but it's a bit tricky.
> 
>> Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
>>
>> On 11/24/2017 06:31 PM, Aníbal Limón wrote:
>>>
>>> From: Aníbal Limón <anibal.limon@linaro.org>
>>>
>>> Some EGL implementations do not actually ship all Khronos-extensions.
>>> As it turns out, the Mali 450 driver does not include
>>> eglGetPlatformDisplay
>>> symbol so there is not grauntee to exists use
>>> piglit_egl_get_default_display
>>
>>
>> This needs to be rewritten with proper English, not sure how. Maybe simply:
>>
>> "Mali 450 driver does not support eglGetPlatformDisplay, use
>> piglit_egl_get_default_display wrapper instead."
>>
>>
> Looks good, one could also reuse the text from 45095dc08b.
> In either case, patch is
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> 
> -Emil
>

Patch

diff --git a/tests/egl/egl-context-priority.c b/tests/egl/egl-context-priority.c
index 7f26fc614..9590ccae3 100644
--- a/tests/egl/egl-context-priority.c
+++ b/tests/egl/egl-context-priority.c
@@ -99,8 +99,7 @@  piglit_init(int argc, char **argv)
 	if (!strstr(exts, "EGL_MESA_platform_surfaceless"))
 		piglit_report_result(PIGLIT_SKIP);
 
-	dpy = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA,
-				    EGL_DEFAULT_DISPLAY, NULL);
+	dpy = piglit_egl_get_default_display(EGL_PLATFORM_SURFACELESS_MESA);
 
 	ok = eglInitialize(dpy, &major, &minor);
 	if (!ok) {