On Friday, 2018-08-31 17:59:10 +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.veli...@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.f...@collabora.com>
> Cc: Tomasz Figa <tf...@chromium.org>
> Cc: Mauro Rossi <issor.or...@gmail.com>
> Signed-off-by: Emil Velikov <emil.veli...@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)
> + 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
