On Mon, Sep 3, 2018 at 3:25 PM Tomasz Figa <tf...@chromium.org> wrote:
>
> On Mon, Sep 3, 2018 at 2:45 PM Tomasz Figa <tf...@chromium.org> wrote:
> >
> > Hi Emil,
> >
> > On Sat, Sep 1, 2018 at 2:03 AM Emil Velikov <emil.l.veli...@gmail.com>
> > wrote:
> > >
> > > From: Emil Velikov <emil.veli...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
