[Mesa-dev] [PATCH 6/5] egl/android: continue to next device if dri2_create_screen fails

[Emil Velikov]

On Thu, 23 Aug 2018 at 04:27, Tomasz Figa <tfiga at chromium.org> wrote:
>
> On Thu, Aug 23, 2018 at 1:44 AM Emil Velikov <emil.l.velikov at gmail.com> wrote:
> >
> > Hi Tomasz,
> >
> > On 21 August 2018 at 14:54, Tomasz Figa <tfiga at chromium.org> wrote:
> > > Hi Emil,
> > >
> > > On Tue, Aug 14, 2018 at 2:05 AM Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > >>
> > >> From: Emil Velikov <emil.velikov at collabora.com>
> > >>
> > >> Unlike the other platforms, here we aim do guess if the device that we
> > >> somewhat arbitrarily picked, is supported or not.
> > >>
> > >> It seems a bit fiddly, but considering EGL_EXT_explicit_device and
> > >> EGL_MESA_query_renderer are MIA, this is the best we can do for the
> > >> moment.
> > >>
> > >> With those (proposed) extensions userspace will be able to create a
> > >> separate EGL display for each device, query device details and make the
> > >> conscious decision which one to use.
> > >>
> > >> Cc: Robert Foss <robert.foss at collabora.com>
> > >> Cc: Tomasz Figa <tfiga at chromium.org>
> > >> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> > >> ---
> > >>  src/egl/drivers/dri2/platform_android.c | 29 ++++++++++++++++---------
> > >>  1 file changed, 19 insertions(+), 10 deletions(-)
> > >>
> > >> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> > >> index 50dd7a5e1b4..cac59847b89 100644
> > >> --- a/src/egl/drivers/dri2/platform_android.c
> > >> +++ b/src/egl/drivers/dri2/platform_android.c
> > >> @@ -1295,6 +1295,25 @@ droid_open_device(_EGLDisplay *disp)
> > >>           continue;
> > >>        }
> > >>        /* Found a device */
> > >> +
> > >> +      /* Check that the device is supported, by attempting to:
> > >> +       * - load the dri module
> > >> +       * - and, create a screen
> > >> +       */
> > >> +      if (!droid_load_driver(disp)) {
> > >> +         _eglLog(_EGL_WARNING, "DRI2: failed to load driver");
> > >> +         close(fd);
> > >> +         fd = -1;
> > >> +         continue;
> > >> +      }
> > >> +
> > >> +      if (!dri2_create_screen(disp)) {
> > >> +         _eglLog(_EGL_WARNING, "DRI2: failed to create screen");
> > >> +         close(fd);
> > >> +         fd = -1;
> > >> +         continue;
> > >> +      }
> > >
> > > Don't we also need to do these tests when determining if the device is
> > > a suitable fallback? The fallback fd is set much earlier, in the same
> > > block as the continue statement, so the code below wouldn't execute.
> > >
> > Let me see if I got this correctly:
> >  - when a "vendor" is requested we use that, falling back to the first
> > other driver where screen creation succeeds
> >  - if one isn't requested, we pick the first device that can create a screen
> >
> > Is that right?
>
> Yes, seems to match my idea.
>
> Just to make sure we're not going to be masking some failures with
> fallbacks, I think we should make sure that if "vendor" is requested
> and found, we don't fallback, even if the matched device fails to
> create a screen.
>
The original code _is_ using a fallback when a vendor is requested,
which got to me.
Thanks a lot for the clarification. Updated patch should be in your inbox + ML.

Cheers,
Emil

[1] https://lists.freedesktop.org/archives/mesa-dev/2018-August/203533.html