[Mesa-dev] [PATCH v2] egl/android: rework device probing
Eric Engestrom
eric.engestrom at intel.com
Mon Sep 3 07:47:57 UTC 2018
On Monday, 2018-09-03 08:40:47 +0100, Eric Engestrom wrote:
> On Friday, 2018-08-31 17:59:10 +0100, Emil Velikov 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.
> >
> > In particular: when a vendor is _not_ requested we loop through all
> > devices, picking the first one which can create a DRI screen.
> >
> > When a vendor is requested - we use that and do _not_ fall-back to any
> > other device.
> >
> > The former 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.
> >
> > v2:
> > - update droid_open_device_drm_gralloc()
> > - set the dri2_dpy->fd before using it
> > - return a EGLBoolean for droid_{probe,open}_device*
> > - do not warn on droid_load_driver failure (Tomasz)
> > - plug mem leak on dri2_create_screen failure (Tomasz)
> > - fixup function name typo (Tomasz, Rob)
> >
> > Cc: Robert Foss <robert.foss at collabora.com>
> > Cc: Tomasz Figa <tfiga at chromium.org>
> > Cc: Mauro Rossi <issor.oruam at gmail.com>
> > Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> > ---
> > Rob, I choose not to keep your r-b since the patch has changed quite a
> > bit.
> >
> > Mauro, please check that this version doesn't break the drm_gralloc case.
> >
> > Thanks
> > Emil
> > ---
> > src/egl/drivers/dri2/platform_android.c | 116 +++++++++++++++---------
> > 1 file changed, 73 insertions(+), 43 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> > index 1f9fe27ab85..0586633a6db 100644
> > --- a/src/egl/drivers/dri2/platform_android.c
> > +++ b/src/egl/drivers/dri2/platform_android.c
> > @@ -1203,9 +1203,10 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy)
> > }
> >
> > #ifdef HAVE_DRM_GRALLOC
> > -static int
> > -droid_open_device_drm_gralloc(struct dri2_egl_display *dri2_dpy)
> > +static EGLBoolean
> > +droid_open_device_drm_gralloc(_EGLDisplay *disp)
> > {
> > + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> > int fd = -1, err = -EINVAL;
> >
> > if (dri2_dpy->gralloc->perform)
> > @@ -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 is doing `dpy->fd = (dup() < 0)`.
>
> You could add parentheses to fix it, or simply split into
> dpy->fd = dup();
> if (dpy->fd < 0)
>
> (voting for the latter)
Sorry, ignore this, I just saw that Tomasz pointed it out as well :]
(I should really read other people's replies before saying anything)
>
> > + 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)
> > {
> > - 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;
> > }
> >
> > +static void
> > +droid_unload_driver(_EGLDisplay *disp)
> > +{
> > + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> > +
> > + dlclose(dri2_dpy->driver);
> > + dri2_dpy->driver = NULL;
> > + free(dri2_dpy->driver_name);
> > + dri2_dpy->driver_name = NULL;
> > +}
> > +
> > static int
> > droid_filter_device(_EGLDisplay *disp, int fd, const char *vendor)
> > {
> > @@ -1420,13 +1435,31 @@ droid_filter_device(_EGLDisplay *disp, int fd, const char *vendor)
> > return 0;
> > }
> >
> > -static int
> > +static EGLBoolean
> > +droid_probe_device(_EGLDisplay *disp)
> > +{
> > + /* Check that the device is supported, by attempting to:
> > + * - load the dri module
> > + * - and, create a screen
> > + */
> > + if (!droid_load_driver(disp))
> > + return EGL_FALSE;
> > +
> > + if (!dri2_create_screen(disp)) {
> > + _eglLog(_EGL_WARNING, "DRI2: failed to create screen");
> > + droid_unload_driver(disp);
> > + return EGL_FALSE;
> > + }
> > + return EGL_TRUE;
> > +}
> > +
> > +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;
> > - int fd = -1, fallback_fd = -1;
> >
> > char *vendor_name = NULL;
> > char vendor_buf[PROPERTY_VALUE_MAX];
> > @@ -1436,7 +1469,7 @@ droid_open_device(_EGLDisplay *disp)
> >
> > num_devices = drmGetDevices2(0, devices, ARRAY_SIZE(devices));
> > if (num_devices < 0)
> > - return num_devices;
> > + return EGL_FALSE;
> >
> > for (int i = 0; i < num_devices; i++) {
> > device = devices[i];
> > @@ -1444,41 +1477,49 @@ droid_open_device(_EGLDisplay *disp)
> > if (!(device->available_nodes & (1 << DRM_NODE_RENDER)))
> > continue;
> >
> > - fd = loader_open_device(device->nodes[DRM_NODE_RENDER]);
> > - if (fd < 0) {
> > + dri2_dpy->fd = loader_open_device(device->nodes[DRM_NODE_RENDER]);
> > + if (dri2_dpy->fd < 0) {
> > _eglLog(_EGL_WARNING, "%s() Failed to open DRM device %s",
> > __func__, device->nodes[DRM_NODE_RENDER]);
> > continue;
> > }
> >
> > - if (vendor_name && droid_filter_device(disp, fd, vendor_name)) {
> > - /* Match requested, but not found - set as fallback */
> > - if (fallback_fd == -1) {
> > - fallback_fd = fd;
> > - } else {
> > - close(fd);
> > - fd = -1;
> > + /* If a vendor is explicitly provided, we use only that.
> > + * Otherwise we fall-back the first device that is supported.
> > + */
> > + if (vendor_name) {
> > + if (droid_filter_device(disp, dri2_dpy->fd, vendor_name)) {
> > + /* Device does not match - try next device */
> > + close(dri2_dpy->fd);
> > + dri2_dpy->fd = -1;
> > + continue;
> > + }
> > + /* If the requested device matches - use it. Regardless if
> > + * init fails, do not fall-back to any other device.
> > + */
> > + if (!droid_probe_device(disp)) {
> > + close(dri2_dpy->fd);
> > + dri2_dpy->fd = -1;
> > }
> >
> > - continue;
> > + break;
> > }
> > - /* Found a device */
> > - break;
> > - }
> > - drmFreeDevices(devices, num_devices);
> > + if (droid_probe_device(disp))
> > + break;
> >
> > - if (fallback_fd < 0 && fd < 0) {
> > - _eglLog(_EGL_WARNING, "Failed to open any DRM device");
> > - return -1;
> > + /* No explicit request - attempt the next device */
> > + close(dri2_dpy->fd);
> > + dri2_dpy->fd = -1;
> > }
> > + drmFreeDevices(devices, num_devices);
> >
> > - if (fd < 0) {
> > - _eglLog(_EGL_WARNING, "Failed to open desired DRM device, using fallback");
> > - return fallback_fd;
> > + if (dri2_dpy->fd < 0) {
> > + _eglLog(_EGL_WARNING, "Failed to open %s DRM device",
> > + vendor_name ? "desired": "any");
> > + return EGL_FALSE;
> > }
> >
> > - close(fallback_fd);
> > - return fd;
> > + return EGL_TRUE;
> > #undef MAX_DRM_DEVICES
> > }
> >
> > @@ -1510,25 +1551,14 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp)
> > disp->DriverData = (void *) dri2_dpy;
> >
> > #ifdef HAVE_DRM_GRALLOC
> > - dri2_dpy->fd = droid_open_device_drm_gralloc(dri2_dpy);
> > + if (!droid_open_device_drm_gralloc(disp)) {
> > #else
> > - dri2_dpy->fd = droid_open_device(disp);
> > + if (!droid_open_device(disp)) {
> > #endif
> > - if (dri2_dpy->fd < 0) {
> > err = "DRI2: failed to open device";
> > goto cleanup;
> > }
> >
> > - if (!droid_load_driver(disp)) {
> > - err = "DRI2: failed to load driver";
> > - goto cleanup;
> > - }
> > -
> > - if (!dri2_create_screen(disp)) {
> > - err = "DRI2: failed to create screen";
> > - goto cleanup;
> > - }
> > -
> > if (!dri2_setup_extensions(disp)) {
> > err = "DRI2: failed to setup extensions";
> > goto cleanup;
> > --
> > 2.18.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list