[Mesa-dev] [PATCH v2] egl/android: rework device probing

[Tomasz Figa]

On Mon, Sep 3, 2018 at 3:25 PM Tomasz Figa <tfiga at chromium.org> wrote:
>
> On Mon, Sep 3, 2018 at 2:45 PM Tomasz Figa <tfiga at chromium.org> wrote:
> >
> > Hi Emil,
> >
> > On Sat, Sep 1, 2018 at 2:03 AM Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > >
> > > From: Emil Velikov <emil.velikov at collabora.com>
> > >
> >
> > Thanks for the patch! Please see my comments below.
> >
> > [snip]
> > > @@ -1214,10 +1215,13 @@ droid_open_device_drm_gralloc(struct dri2_egl_display *dri2_dpy)
> > >                                            &fd);
> > >     if (err || fd < 0) {
> > >        _eglLog(_EGL_WARNING, "fail to get drm fd");
> > > -      fd = -1;
> > > +      return EGL_FALSE;
> > >     }
> > >
> > > -   return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> > > +   if (dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3) < 0)
> >
> > This will assign the boolean result of the comparison to dri2_dpy->fd.
> > To avoid parenthesis hell, I'd rewrite this to:
> >
> > dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
> > if (dri2_dpy->fd < 0)
> >    return EGL_FALSE;
> >
> > > +     return EGL_FALSE;
> > > +
> > > +   return droid_probe_device(disp);
> > >  }
> > >  #endif /* HAVE_DRM_GRALLOC */
> > >
> > > @@ -1365,7 +1369,7 @@ static const __DRIextension *droid_image_loader_extensions[] = {
> > >  EGLBoolean
> > >  droid_load_driver(_EGLDisplay *disp)
> >
> > Not related to this patch, but I guess we could fix it up, while at
> > it. Fails to build with:
> >
> > src/egl/drivers/dri2/platform_android
> > .c:1369:1: error: no previous prototype for function
> > 'droid_load_driver' [-Werror,-Wmissing-prototypes]
> > droid_load_driver(_EGLDisplay *disp)
> > ^
> >
> > The function should be static.
> >
> > >  {
> > > -   struct dri2_egl_display *dri2_dpy = disp->DriverData;
> > > +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> > >     const char *err;
> > >
> > >     dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> > > @@ -1404,6 +1408,17 @@ error:
> > >     return false;
> > >  }
> > [snip]
> > > +static EGLBoolean
> > >  droid_open_device(_EGLDisplay *disp)
> > >  {
> > >  #define MAX_DRM_DEVICES 32
> > > +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> > >     drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL };
> > >     int prop_set, num_devices;
> >
> > Not related to this patch, but prop_set is unused. We could add a
> > fixup in this patch, while reworking this already.
> >
> > I'm going to test it on Chrome OS with the fixups above applied.
>
> The probing itself seems to be working fine, but it looks like there
> is some EGL image regression in master, which I don't have time to
> investigate.

Never mind, it looks like I had libdrm in some weird state. After a
complete clean build, it worked fine.

Best regards,
Tomasz