[Mesa-dev] [PATCH] egl/dri2: dri2_make_current: Set EGL error if bindContext fails

[Nicolas Boichat]

Hi,

On Wed, Jul 13, 2016 at 11:21 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 7 June 2016 at 11:14, Nicolas Boichat <drinkcat at chromium.org> wrote:
>> Without this, if a configuration is, say, available only on GLES2/3, but
>> not on GLES1, eglMakeCurrent fails with error "EGL_SUCCESS".
>>
>> In this patch, we set error to EGL_BAD_MATCH, which is what CTS/dEQP
>> dEQP-EGL.functional.surfaceless_context expect.
>>
> Since all the EGL_KHR_surfaceless_context particulars are/should be
> handled by _eglBindContext(_eglCheckMakeCurrent actually), this patch
> covers the case when the dri module fails in bindContext(), correct ?
> Can you please mention that in the commit message.

Yes that's correct, the problem here is a failure in the dri module,
and, since it's a dri module we can't report the EGL error there.

> Please add the stable tag:
> Cc: "11.2 12.0" <mesa-stable at lists.freedesktop.org>
>
>> Signed-off-by: Nicolas Boichat <drinkcat at google.com>
>> ---
>>  src/egl/drivers/dri2/egl_dri2.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
>> index bfde640..1a38421 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.c
>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>> @@ -1231,6 +1231,7 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf,
>>        _eglPutSurface(old_rsurf);
>>        _eglPutContext(old_ctx);
>>
>> +      _eglError(EGL_BAD_MATCH, "bindContext error");
> Please use "eglMakeCurrent" as error string.
>
> Related: the error paths looks a bit confusing so any ideas how to
> untangle this will be appreciated. Not a requirement for this to land
> though.

Yes, and we are hitting other issues with this specific function, when
dri2_dpy is NULL (if you call eglTerminate() and then
eglReleaseThread(), like some CTS tests do). So we'll need to add more
sanity checks, and error handling will need to be reworked. We'll look
at it in a follow-up patch.

> Considering my understanding is correct, with the above two
> suggestions the patch is:
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

Will fix and respin, thanks.

Best,