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

2018-09-20 Thread Emil Velikov 
On 10 September 2018 at 22:23, Mauro Rossi  wrote:
> Hi Emil,
> Il giorno lun 3 set 2018 alle ore 14:42 Emil Velikov
>  ha scritto:
>>
>> From: Emil Velikov 
>>
>> 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)
>>
>> v3:
>>  - add forward declaration for droid_load_driver()
>> Fixes the HAVE_DRM_GRALLOC build (Mauro)
>>  - split dup() assignment and check in separate lines (Tomasz, Eric)
>>  - make droid_load_driver() static (Tomasz)
>>  - drop unused prop_set variable (Tomasz)
>>
>> Cc: Robert Foss 
>> Cc: Tomasz Figa 
>> Cc: Mauro Rossi 
>> Signed-off-by: Emil Velikov 
>> Tested-by: Tomasz Figa 
>> Tested-by: Tapani Pälli 
>> ---
>> Thanks for the feedback everyone.
>>
>> Mauro, this includes a forward declaration which should fix things for
>> you.
>> ---
>>  src/egl/drivers/dri2/platform_android.c | 124 +++-
>>  1 file changed, 79 insertions(+), 45 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_android.c 
>> b/src/egl/drivers/dri2/platform_android.c
>> index ecc0245c9a2..5a73d9e7ea9 100644
>> --- a/src/egl/drivers/dri2/platform_android.c
>> +++ b/src/egl/drivers/dri2/platform_android.c
>> @@ -1202,10 +1202,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, 
>> _EGLDisplay *dpy)
>> return (config_count != 0);
>>  }
>>
>> +static EGLBoolean
>> +droid_load_driver(_EGLDisplay *disp);
>
> droid_probe_device(_EGLDisplay *disp);
>
> is required here, as it is invoked in droid_open_device_drm_gralloc()
> With this change there is no regression in the drivers I've tested
> (Intel and AMD)
>
Thanks Mauro, tweaked and pushed to master.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-09-10 Thread Mauro Rossi 
Hi Emil,
Il giorno lun 3 set 2018 alle ore 14:42 Emil Velikov
 ha scritto:
>
> From: Emil Velikov 
>
> 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)
>
> v3:
>  - add forward declaration for droid_load_driver()
> Fixes the HAVE_DRM_GRALLOC build (Mauro)
>  - split dup() assignment and check in separate lines (Tomasz, Eric)
>  - make droid_load_driver() static (Tomasz)
>  - drop unused prop_set variable (Tomasz)
>
> Cc: Robert Foss 
> Cc: Tomasz Figa 
> Cc: Mauro Rossi 
> Signed-off-by: Emil Velikov 
> Tested-by: Tomasz Figa 
> Tested-by: Tapani Pälli 
> ---
> Thanks for the feedback everyone.
>
> Mauro, this includes a forward declaration which should fix things for
> you.
> ---
>  src/egl/drivers/dri2/platform_android.c | 124 +++-
>  1 file changed, 79 insertions(+), 45 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c 
> b/src/egl/drivers/dri2/platform_android.c
> index ecc0245c9a2..5a73d9e7ea9 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -1202,10 +1202,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, 
> _EGLDisplay *dpy)
> return (config_count != 0);
>  }
>
> +static EGLBoolean
> +droid_load_driver(_EGLDisplay *disp);

droid_probe_device(_EGLDisplay *disp);

is required here, as it is invoked in droid_open_device_drm_gralloc()
With this change there is no regression in the drivers I've tested
(Intel and AMD)

nouveau has some other problem, manifesting as drm driver errors and
freezes, happening also with drm_gralloc,
but it is not due to this patch, the problem was alreaday present w/o
this patch.

Mauro

> +
>  #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 +1218,14 @@ 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;
> +   dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
> +   if (dri2_dpy->fd < 0)
> + return EGL_FALSE;
> +
> +   return droid_probe_device(disp);
>  }
>  #endif /* HAVE_DRM_GRALLOC */
>
> @@ -1362,10 +1370,10 @@ static const __DRIextension 
> *droid_image_loader_extensions[] = {
> NULL,
>  };
>
> -EGLBoolean
> +static 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 +1412,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 +1439,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

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

2018-09-04 Thread Tomasz Figa 
On Mon, Sep 3, 2018 at 9:42 PM Emil Velikov  wrote:
>
> From: Emil Velikov 
>
> 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)
>
> v3:
>  - add forward declaration for droid_load_driver()
> Fixes the HAVE_DRM_GRALLOC build (Mauro)
>  - split dup() assignment and check in separate lines (Tomasz, Eric)
>  - make droid_load_driver() static (Tomasz)
>  - drop unused prop_set variable (Tomasz)

Thanks a lot!

Reviewed-by: Tomasz Figa 

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-09-03 Thread Emil Velikov 
From: Emil Velikov 

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)

v3:
 - add forward declaration for droid_load_driver()
Fixes the HAVE_DRM_GRALLOC build (Mauro)
 - split dup() assignment and check in separate lines (Tomasz, Eric)
 - make droid_load_driver() static (Tomasz)
 - drop unused prop_set variable (Tomasz)

Cc: Robert Foss 
Cc: Tomasz Figa 
Cc: Mauro Rossi 
Signed-off-by: Emil Velikov 
Tested-by: Tomasz Figa 
Tested-by: Tapani Pälli 
---
Thanks for the feedback everyone.

Mauro, this includes a forward declaration which should fix things for
you.
---
 src/egl/drivers/dri2/platform_android.c | 124 +++-
 1 file changed, 79 insertions(+), 45 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index ecc0245c9a2..5a73d9e7ea9 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -1202,10 +1202,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, 
_EGLDisplay *dpy)
return (config_count != 0);
 }
 
+static EGLBoolean
+droid_load_driver(_EGLDisplay *disp);
+
 #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 +1218,14 @@ 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;
+   dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+   if (dri2_dpy->fd < 0)
+ return EGL_FALSE;
+
+   return droid_probe_device(disp);
 }
 #endif /* HAVE_DRM_GRALLOC */
 
@@ -1362,10 +1370,10 @@ static const __DRIextension 
*droid_image_loader_extensions[] = {
NULL,
 };
 
-EGLBoolean
+static 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 +1412,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 +1439,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;
+   int num_devices;
 
char *vendor_name = NULL;
char vendor_buf[PROPERTY_VALUE_MAX];
@@ -1436,7 +1473,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 +1481,49 @@ droid_open_device(_EGLDisplay *disp)
   if (!(device->available_nodes & (1 << DRM_NODE_RENDER)))
  continue;
 
-  fd = loader_open_device(