Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
Hey Tomasz,
Thanks for the quick feedback.
On 2018-06-14 08:30, Tomasz Figa wrote:
Hi Rob,
Thanks for sending v3. Please see few more comments inline.
On Sun, Jun 10, 2018 at 2:28 AM Robert Foss wrote:
This patch both adds support for probing & filtering DRM nodes
and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD
gralloc call.
Currently the filtering is based just on the driver name,
and the desired name is supplied using the "drm.gpu.vendor_name"
Android property.
Signed-off-by: Robert Foss
---
Changes since v2:
- Switch from drmGetDevices2 to manual renderD node iteration
- Add probe_res enum to communicate probing results better
- Avoid using _eglError() in internal static functions
- Avoid actually loading the driver while probing, just verify
that it exists.
- Replace strlen call with the assumed length PROPERTY_VALUE_MAX
[snip]
+static probe_ret_t
+droid_probe_device(_EGLDisplay *disp, int fd, char *vendor)
+{
+ int ret;
+
+ drmVersionPtr ver = drmGetVersion(fd);
+ if (!ver)
+ return probe_error;
+
+ if (vendor != NULL && ver->name != NULL &&
+ strncmp(vendor, ver->name, PROPERTY_VALUE_MAX) != 0) {
Shouldn't we fail the match if vendor != NULL, but ver->name == NULL? i.e.
if (vendor && (!ver->name || strcmp(...)) { ...
Yeah, overall that if-case is too much. I'll split out the NULL check separate
check that return an error.
+ ret = probe_filtered_out;
+ goto cleanup;
+ }
+
+
+ if (!droid_probe_driver(fd)) {
+ ret = probe_no_driver;
+ goto cleanup;
+ }
+
+ ret = probe_success;
+
+cleanup:
+ drmFreeVersion(ver);
+ return ret;
+}
+
+static int
+droid_open_device(_EGLDisplay *disp)
+{
+ const int MAX_DRM_DEVICES = 32;
+ int prop_set, num_devices;
+ int fd = -1, fallback_fd = -1;
+
+ char *vendor_name = NULL;
+ char vendor_buf[PROPERTY_VALUE_MAX];
+ if (property_get("drm.gpu.vendor_name", vendor_buf, NULL) > 0);
+ vendor_name = vendor_buf;
+
+ const char *drm_dir_name = "/dev/dri";
+ DIR *sysdir = opendir(drm_dir_name);
+ if (!sysdir)
+ return -errno;
+
+ struct dirent *dent;
+ while ((dent = readdir(sysdir))) {
+ char dev_path[128];
+ char *render_dev_prefix = "renderD";
+ size_t prefix_len = strlen(render_dev_prefix);
If a const array like below is used instead
const char render_dev_prefix[] = "renderD";
you can just use sizeof(render_dev_prefix), without the need for strlen().
Ack.
+
+ if (strncmp(render_dev_prefix, dent->d_name, prefix_len) != 0)
+ continue;
+
+ sprintf(dev_path, "%s/%s", drm_dir_name, dent->d_name);
snprintf(dev_path, sizeof(dev_path), ...);
Ack.
+ fd = loader_open_device(dev_path);
+ if (fd == -1) {
nit: Perhaps fd < 0, just to be safe?
Ack.
+ _eglLog(_EGL_WARNING, "%s() Failed to open DRM device %s",
+ __func__, dev_path);
+ continue;
+ }
+
+ int ret = droid_probe_device(disp, fd, vendor_name);
+ switch (ret) {
+ case probe_success:
+ goto success;
+ case probe_filtered_out:
+ goto allow_fallback;
The 2 lines of code at the "allow_fallback" label could be just moved here.
Ack.
+ case probe_error:
+ case probe_no_driver:
Do we need 2 separate cases for these? Just one "probe_fail" should be enough.
Ack.
+ goto next;
If we move the fallback handling to "probe_filtered_out" case, we
could remove the "next" label too and simply "break" here.
Ack.
+ }
+
+allow_fallback:
+ if (fallback_fd == -1)
+ fallback_fd = fd;
+next:
+ if (fallback_fd != fd)
+ close(fd);
+ fd = -1;
+ continue;
+ }
+
+success:
+ closedir(sysdir);
+
+ if (fallback_fd < 0 && fd < 0) {
+ _eglLog(_EGL_WARNING, "Failed to open any DRM device");
+ return -1;
+ }
+
+ if (fd < 0) {
+ _eglLog(_EGL_WARNING, "Failed to open desired DRM device, using
fallback");
+ return fallback_fd;
+ }
+
+ close(fallback_fd);
+ return fd;
+}
[Leaving context for readability.]
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
On 2018-06-14 04:54, Tomasz Figa wrote: On Thu, Jun 14, 2018 at 4:14 AM Rob Herring wrote: On Wed, Jun 13, 2018 at 12:19 PM, Amit Pundir wrote: On 13 June 2018 at 20:45, Rob Herring wrote: +Amit and John On Sat, Jun 9, 2018 at 11:27 AM, Robert Foss wrote: This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. There's a potential issue with this whole approach and that is SELinux. With the way SELinux locks down accesses, getting probing thru device files to work can be a pain. It may be better now than the prior version because sysfs is not probed. I'll leave it to Amit or John to comment. Right.. so ICYMI, this patch is already pulled into external/mesa3d project of AOSP and I stumbled upon one such /dev/dri/ access denial on db820c recently. A prior version of the patch series which accesses sysfs too (via libdrm). In AOSP, zygote spawned apps already have access to GPU device nodes in the form of /dev/gpu_device file, but the missing part is the It's "gpu_device" in terms a a SELinux context, right? Not an actual /dev path? open-read access to "/dev/dri/" which need to be allowed explicitly. Or we need a way to just open a specific device. How does this work in drm_gralloc (or any other existing gralloc that implements GRALLOC_MODULE_PERFORM_GET_DRM_FD)? What device node does it open? I have some loose recollection that it had a property specifying exact device. I guess we could use that property as the top level override, bypassing the whole probing logic, if present. drm_gralloc in aosp/master uses the "gralloc.drm.device" property. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
On 14 June 2018 at 00:44, Rob Herring wrote: > On Wed, Jun 13, 2018 at 12:19 PM, Amit Pundir wrote: >> On 13 June 2018 at 20:45, Rob Herring wrote: >>> >>> +Amit and John >>> >>> On Sat, Jun 9, 2018 at 11:27 AM, Robert Foss >>> wrote: >>> > This patch both adds support for probing & filtering DRM nodes >>> > and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD >>> > gralloc call. >>> > >>> > Currently the filtering is based just on the driver name, >>> > and the desired name is supplied using the "drm.gpu.vendor_name" >>> > Android property. >>> >>> There's a potential issue with this whole approach and that is >>> SELinux. With the way SELinux locks down accesses, getting probing >>> thru device files to work can be a pain. It may be better now than the >>> prior version because sysfs is not probed. I'll leave it to Amit or >>> John to comment. >> >> Right.. so ICYMI, this patch is already pulled into external/mesa3d >> project of AOSP and I stumbled upon one such /dev/dri/ access denial >> on db820c recently. > > A prior version of the patch series which accesses sysfs too (via libdrm). > >> >> In AOSP, zygote spawned apps already have access to GPU device nodes >> in the form of /dev/gpu_device file, but the missing part is the > > It's "gpu_device" in terms a a SELinux context, right? Not an actual /dev > path? Yes in SELinux context, it is not an actual /dev/ path. Regards, Amit Pundir > >> open-read access to "/dev/dri/" which need to be allowed explicitly. > > Or we need a way to just open a specific device. > > Rob ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
On 13 June 2018 at 20:45, Rob Herring wrote:
>
> +Amit and John
>
> On Sat, Jun 9, 2018 at 11:27 AM, Robert Foss
> wrote:
> > This patch both adds support for probing & filtering DRM nodes
> > and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD
> > gralloc call.
> >
> > Currently the filtering is based just on the driver name,
> > and the desired name is supplied using the "drm.gpu.vendor_name"
> > Android property.
>
> There's a potential issue with this whole approach and that is
> SELinux. With the way SELinux locks down accesses, getting probing
> thru device files to work can be a pain. It may be better now than the
> prior version because sysfs is not probed. I'll leave it to Amit or
> John to comment.
Right.. so ICYMI, this patch is already pulled into external/mesa3d
project of AOSP and I stumbled upon one such /dev/dri/ access denial
on db820c recently.
In AOSP, zygote spawned apps already have access to GPU device nodes
in the form of /dev/gpu_device file, but the missing part is the
open-read access to "/dev/dri/" which need to be allowed explicitly.
Rest of the denials related to sysfs access can be easily resolved
using audit2allow tool.
Regards,
Amit Pundir
>
> Rob
>
> >
> > Signed-off-by: Robert Foss
> > ---
> >
> > Changes since v2:
> > - Switch from drmGetDevices2 to manual renderD node iteration
> > - Add probe_res enum to communicate probing results better
> > - Avoid using _eglError() in internal static functions
> > - Avoid actually loading the driver while probing, just verify
> >that it exists.
> > - Replace strlen call with the assumed length PROPERTY_VALUE_MAX
> >
> > Changes since v1:
> > - Do not rely on libdrm for probing
> > - Distinguish between errors and when no drm devices are found
> >
> > Changes since RFC:
> > - Rebased on newer libdrm drmHandleMatch patch
> > - Added support for driver probing
> >
> >
> > src/egl/drivers/dri2/platform_android.c | 222 ++--
> > 1 file changed, 169 insertions(+), 53 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_android.c
> > b/src/egl/drivers/dri2/platform_android.c
> > index 4ba96aad90..a2cbe92d93 100644
> > --- a/src/egl/drivers/dri2/platform_android.c
> > +++ b/src/egl/drivers/dri2/platform_android.c
> > @@ -27,12 +27,16 @@
> > * DEALINGS IN THE SOFTWARE.
> > */
> >
> > +#include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > +#include
> >
> > #include "loader.h"
> > #include "egl_dri2.h"
> > @@ -1130,31 +1134,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
> > _EGLDisplay *dpy)
> > return (config_count != 0);
> > }
> >
> > -enum {
> > -/* perform(const struct gralloc_module_t *mod,
> > - * int op,
> > - * int *fd);
> > - */
> > -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002,
> > -};
> > -
> > -static int
> > -droid_open_device(struct dri2_egl_display *dri2_dpy)
> > -{
> > - int fd = -1, err = -EINVAL;
> > -
> > - if (dri2_dpy->gralloc->perform)
> > - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc,
> > -
> > GRALLOC_MODULE_PERFORM_GET_DRM_FD,
> > - &fd);
> > - if (err || fd < 0) {
> > - _eglLog(_EGL_WARNING, "fail to get drm fd");
> > - fd = -1;
> > - }
> > -
> > - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> > -}
> > -
> > static const struct dri2_egl_display_vtbl droid_display_vtbl = {
> > .authenticate = NULL,
> > .create_window_surface = droid_create_window_surface,
> > @@ -1215,6 +1194,168 @@ static const __DRIextension
> > *droid_image_loader_extensions[] = {
> > NULL,
> > };
> >
> > +EGLBoolean
> > +droid_load_driver(_EGLDisplay *disp)
> > +{
> > + struct dri2_egl_display *dri2_dpy = disp->DriverData;
> > + const char *err;
> > +
> > + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> > + if (dri2_dpy->driver_name == NULL)
> > + return false;
> > +
> > + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) ==
> > DRM_NODE_RENDER;
> > +
> > + if (!dri2_dpy->is_render_node) {
> > + #ifdef HAVE_DRM_GRALLOC
> > + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM
> > names
> > +* for backwards compatibility with drm_gralloc. (Do not use on new
> > +* systems.) */
> > + dri2_dpy->loader_extensions = droid_dri2_loader_extensions;
> > + if (!dri2_load_driver(disp)) {
> > + err = "DRI2: failed to load driver";
> > + goto error;
> > + }
> > + #else
> > + err = "DRI2: handle is not for a render node";
> > + goto error;
> > + #endif
> > + } else {
> > + dri2_dpy->loader_extensions = droid_image_loader_extensions;
> > + if (!dri2_load_driver_dri3(disp)) {
> > + err = "DRI3: failed to load driver";
> > + goto error
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
Hi Rob,
Thanks for sending v3. Please see few more comments inline.
On Sun, Jun 10, 2018 at 2:28 AM Robert Foss wrote:
>
> This patch both adds support for probing & filtering DRM nodes
> and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD
> gralloc call.
>
> Currently the filtering is based just on the driver name,
> and the desired name is supplied using the "drm.gpu.vendor_name"
> Android property.
>
> Signed-off-by: Robert Foss
> ---
>
> Changes since v2:
> - Switch from drmGetDevices2 to manual renderD node iteration
> - Add probe_res enum to communicate probing results better
> - Avoid using _eglError() in internal static functions
> - Avoid actually loading the driver while probing, just verify
>that it exists.
> - Replace strlen call with the assumed length PROPERTY_VALUE_MAX
[snip]
> +static probe_ret_t
> +droid_probe_device(_EGLDisplay *disp, int fd, char *vendor)
> +{
> + int ret;
> +
> + drmVersionPtr ver = drmGetVersion(fd);
> + if (!ver)
> + return probe_error;
> +
> + if (vendor != NULL && ver->name != NULL &&
> + strncmp(vendor, ver->name, PROPERTY_VALUE_MAX) != 0) {
Shouldn't we fail the match if vendor != NULL, but ver->name == NULL? i.e.
if (vendor && (!ver->name || strcmp(...)) { ...
> + ret = probe_filtered_out;
> + goto cleanup;
> + }
> +
> +
> + if (!droid_probe_driver(fd)) {
> + ret = probe_no_driver;
> + goto cleanup;
> + }
> +
> + ret = probe_success;
> +
> +cleanup:
> + drmFreeVersion(ver);
> + return ret;
> +}
> +
> +static int
> +droid_open_device(_EGLDisplay *disp)
> +{
> + const int MAX_DRM_DEVICES = 32;
> + int prop_set, num_devices;
> + int fd = -1, fallback_fd = -1;
> +
> + char *vendor_name = NULL;
> + char vendor_buf[PROPERTY_VALUE_MAX];
> + if (property_get("drm.gpu.vendor_name", vendor_buf, NULL) > 0);
> + vendor_name = vendor_buf;
> +
> + const char *drm_dir_name = "/dev/dri";
> + DIR *sysdir = opendir(drm_dir_name);
> + if (!sysdir)
> + return -errno;
> +
> + struct dirent *dent;
> + while ((dent = readdir(sysdir))) {
> + char dev_path[128];
> + char *render_dev_prefix = "renderD";
> + size_t prefix_len = strlen(render_dev_prefix);
If a const array like below is used instead
const char render_dev_prefix[] = "renderD";
you can just use sizeof(render_dev_prefix), without the need for strlen().
> +
> + if (strncmp(render_dev_prefix, dent->d_name, prefix_len) != 0)
> + continue;
> +
> + sprintf(dev_path, "%s/%s", drm_dir_name, dent->d_name);
snprintf(dev_path, sizeof(dev_path), ...);
> + fd = loader_open_device(dev_path);
> + if (fd == -1) {
nit: Perhaps fd < 0, just to be safe?
> + _eglLog(_EGL_WARNING, "%s() Failed to open DRM device %s",
> + __func__, dev_path);
> + continue;
> + }
> +
> + int ret = droid_probe_device(disp, fd, vendor_name);
> + switch (ret) {
> + case probe_success:
> + goto success;
> + case probe_filtered_out:
> + goto allow_fallback;
The 2 lines of code at the "allow_fallback" label could be just moved here.
> + case probe_error:
> + case probe_no_driver:
Do we need 2 separate cases for these? Just one "probe_fail" should be enough.
> + goto next;
If we move the fallback handling to "probe_filtered_out" case, we
could remove the "next" label too and simply "break" here.
> + }
> +
> +allow_fallback:
> + if (fallback_fd == -1)
> + fallback_fd = fd;
> +next:
> + if (fallback_fd != fd)
> + close(fd);
> + fd = -1;
> + continue;
> + }
> +
> +success:
> + closedir(sysdir);
> +
> + if (fallback_fd < 0 && fd < 0) {
> + _eglLog(_EGL_WARNING, "Failed to open any DRM device");
> + return -1;
> + }
> +
> + if (fd < 0) {
> + _eglLog(_EGL_WARNING, "Failed to open desired DRM device, using
> fallback");
> + return fallback_fd;
> + }
> +
> + close(fallback_fd);
> + return fd;
> +}
[Leaving context for readability.]
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
On Thu, Jun 14, 2018 at 4:14 AM Rob Herring wrote: > > On Wed, Jun 13, 2018 at 12:19 PM, Amit Pundir wrote: > > On 13 June 2018 at 20:45, Rob Herring wrote: > >> > >> +Amit and John > >> > >> On Sat, Jun 9, 2018 at 11:27 AM, Robert Foss > >> wrote: > >> > This patch both adds support for probing & filtering DRM nodes > >> > and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD > >> > gralloc call. > >> > > >> > Currently the filtering is based just on the driver name, > >> > and the desired name is supplied using the "drm.gpu.vendor_name" > >> > Android property. > >> > >> There's a potential issue with this whole approach and that is > >> SELinux. With the way SELinux locks down accesses, getting probing > >> thru device files to work can be a pain. It may be better now than the > >> prior version because sysfs is not probed. I'll leave it to Amit or > >> John to comment. > > > > Right.. so ICYMI, this patch is already pulled into external/mesa3d > > project of AOSP and I stumbled upon one such /dev/dri/ access denial > > on db820c recently. > > A prior version of the patch series which accesses sysfs too (via libdrm). > > > > > In AOSP, zygote spawned apps already have access to GPU device nodes > > in the form of /dev/gpu_device file, but the missing part is the > > It's "gpu_device" in terms a a SELinux context, right? Not an actual /dev > path? > > > open-read access to "/dev/dri/" which need to be allowed explicitly. > > Or we need a way to just open a specific device. How does this work in drm_gralloc (or any other existing gralloc that implements GRALLOC_MODULE_PERFORM_GET_DRM_FD)? What device node does it open? I have some loose recollection that it had a property specifying exact device. I guess we could use that property as the top level override, bypassing the whole probing logic, if present. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
On Wed, Jun 13, 2018 at 12:19 PM, Amit Pundir wrote: > On 13 June 2018 at 20:45, Rob Herring wrote: >> >> +Amit and John >> >> On Sat, Jun 9, 2018 at 11:27 AM, Robert Foss >> wrote: >> > This patch both adds support for probing & filtering DRM nodes >> > and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD >> > gralloc call. >> > >> > Currently the filtering is based just on the driver name, >> > and the desired name is supplied using the "drm.gpu.vendor_name" >> > Android property. >> >> There's a potential issue with this whole approach and that is >> SELinux. With the way SELinux locks down accesses, getting probing >> thru device files to work can be a pain. It may be better now than the >> prior version because sysfs is not probed. I'll leave it to Amit or >> John to comment. > > Right.. so ICYMI, this patch is already pulled into external/mesa3d > project of AOSP and I stumbled upon one such /dev/dri/ access denial > on db820c recently. A prior version of the patch series which accesses sysfs too (via libdrm). > > In AOSP, zygote spawned apps already have access to GPU device nodes > in the form of /dev/gpu_device file, but the missing part is the It's "gpu_device" in terms a a SELinux context, right? Not an actual /dev path? > open-read access to "/dev/dri/" which need to be allowed explicitly. Or we need a way to just open a specific device. Rob ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
+Amit and John
On Sat, Jun 9, 2018 at 11:27 AM, Robert Foss wrote:
> This patch both adds support for probing & filtering DRM nodes
> and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD
> gralloc call.
>
> Currently the filtering is based just on the driver name,
> and the desired name is supplied using the "drm.gpu.vendor_name"
> Android property.
There's a potential issue with this whole approach and that is
SELinux. With the way SELinux locks down accesses, getting probing
thru device files to work can be a pain. It may be better now than the
prior version because sysfs is not probed. I'll leave it to Amit or
John to comment.
Rob
>
> Signed-off-by: Robert Foss
> ---
>
> Changes since v2:
> - Switch from drmGetDevices2 to manual renderD node iteration
> - Add probe_res enum to communicate probing results better
> - Avoid using _eglError() in internal static functions
> - Avoid actually loading the driver while probing, just verify
>that it exists.
> - Replace strlen call with the assumed length PROPERTY_VALUE_MAX
>
> Changes since v1:
> - Do not rely on libdrm for probing
> - Distinguish between errors and when no drm devices are found
>
> Changes since RFC:
> - Rebased on newer libdrm drmHandleMatch patch
> - Added support for driver probing
>
>
> src/egl/drivers/dri2/platform_android.c | 222 ++--
> 1 file changed, 169 insertions(+), 53 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index 4ba96aad90..a2cbe92d93 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -27,12 +27,16 @@
> * DEALINGS IN THE SOFTWARE.
> */
>
> +#include
> #include
> +#include
> #include
> #include
> #include
> #include
> +#include
> #include
> +#include
>
> #include "loader.h"
> #include "egl_dri2.h"
> @@ -1130,31 +1134,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
> _EGLDisplay *dpy)
> return (config_count != 0);
> }
>
> -enum {
> -/* perform(const struct gralloc_module_t *mod,
> - * int op,
> - * int *fd);
> - */
> -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002,
> -};
> -
> -static int
> -droid_open_device(struct dri2_egl_display *dri2_dpy)
> -{
> - int fd = -1, err = -EINVAL;
> -
> - if (dri2_dpy->gralloc->perform)
> - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc,
> - GRALLOC_MODULE_PERFORM_GET_DRM_FD,
> - &fd);
> - if (err || fd < 0) {
> - _eglLog(_EGL_WARNING, "fail to get drm fd");
> - fd = -1;
> - }
> -
> - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> -}
> -
> static const struct dri2_egl_display_vtbl droid_display_vtbl = {
> .authenticate = NULL,
> .create_window_surface = droid_create_window_surface,
> @@ -1215,6 +1194,168 @@ static const __DRIextension
> *droid_image_loader_extensions[] = {
> NULL,
> };
>
> +EGLBoolean
> +droid_load_driver(_EGLDisplay *disp)
> +{
> + struct dri2_egl_display *dri2_dpy = disp->DriverData;
> + const char *err;
> +
> + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> + if (dri2_dpy->driver_name == NULL)
> + return false;
> +
> + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) ==
> DRM_NODE_RENDER;
> +
> + if (!dri2_dpy->is_render_node) {
> + #ifdef HAVE_DRM_GRALLOC
> + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM
> names
> +* for backwards compatibility with drm_gralloc. (Do not use on new
> +* systems.) */
> + dri2_dpy->loader_extensions = droid_dri2_loader_extensions;
> + if (!dri2_load_driver(disp)) {
> + err = "DRI2: failed to load driver";
> + goto error;
> + }
> + #else
> + err = "DRI2: handle is not for a render node";
> + goto error;
> + #endif
> + } else {
> + dri2_dpy->loader_extensions = droid_image_loader_extensions;
> + if (!dri2_load_driver_dri3(disp)) {
> + err = "DRI3: failed to load driver";
> + goto error;
> + }
> +}
> +
> + return true;
> +
> +error:
> + free(dri2_dpy->driver_name);
> + dri2_dpy->driver_name = NULL;
> + return false;
> +}
> +
> +static bool
> +droid_probe_driver(int fd)
> +{
> + char *driver_name;
> +
> + driver_name = loader_get_driver_for_fd(fd);
> + if (driver_name == NULL)
> + return false;
> +
> + free(driver_name);
> + return true;
> +}
> +
> +typedef enum {
> + probe_error = -1,
> + probe_success = 0,
> + probe_filtered_out = 1,
> + probe_no_driver = 2
> +} probe_ret_t;
> +
> +static probe_ret_t
> +droid_probe_device(_EGLDisplay *disp, int fd, char *vendor)
> +{
> + int ret;
> +
> + drmVersionPtr ver = drmGetVersion(fd);
> + if (!ver)
> + return probe_error;
> +
> + if (vendor != NULL && ver->name != NULL &&
> +
[Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
This patch both adds support for probing & filtering DRM nodes
and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD
gralloc call.
Currently the filtering is based just on the driver name,
and the desired name is supplied using the "drm.gpu.vendor_name"
Android property.
Signed-off-by: Robert Foss
---
Changes since v2:
- Switch from drmGetDevices2 to manual renderD node iteration
- Add probe_res enum to communicate probing results better
- Avoid using _eglError() in internal static functions
- Avoid actually loading the driver while probing, just verify
that it exists.
- Replace strlen call with the assumed length PROPERTY_VALUE_MAX
Changes since v1:
- Do not rely on libdrm for probing
- Distinguish between errors and when no drm devices are found
Changes since RFC:
- Rebased on newer libdrm drmHandleMatch patch
- Added support for driver probing
src/egl/drivers/dri2/platform_android.c | 222 ++--
1 file changed, 169 insertions(+), 53 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_android.c
b/src/egl/drivers/dri2/platform_android.c
index 4ba96aad90..a2cbe92d93 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -27,12 +27,16 @@
* DEALINGS IN THE SOFTWARE.
*/
+#include
#include
+#include
#include
#include
#include
#include
+#include
#include
+#include
#include "loader.h"
#include "egl_dri2.h"
@@ -1130,31 +1134,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
_EGLDisplay *dpy)
return (config_count != 0);
}
-enum {
-/* perform(const struct gralloc_module_t *mod,
- * int op,
- * int *fd);
- */
-GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002,
-};
-
-static int
-droid_open_device(struct dri2_egl_display *dri2_dpy)
-{
- int fd = -1, err = -EINVAL;
-
- if (dri2_dpy->gralloc->perform)
- err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc,
- GRALLOC_MODULE_PERFORM_GET_DRM_FD,
- &fd);
- if (err || fd < 0) {
- _eglLog(_EGL_WARNING, "fail to get drm fd");
- fd = -1;
- }
-
- return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
-}
-
static const struct dri2_egl_display_vtbl droid_display_vtbl = {
.authenticate = NULL,
.create_window_surface = droid_create_window_surface,
@@ -1215,6 +1194,168 @@ static const __DRIextension
*droid_image_loader_extensions[] = {
NULL,
};
+EGLBoolean
+droid_load_driver(_EGLDisplay *disp)
+{
+ struct dri2_egl_display *dri2_dpy = disp->DriverData;
+ const char *err;
+
+ dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
+ if (dri2_dpy->driver_name == NULL)
+ return false;
+
+ dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) ==
DRM_NODE_RENDER;
+
+ if (!dri2_dpy->is_render_node) {
+ #ifdef HAVE_DRM_GRALLOC
+ /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names
+* for backwards compatibility with drm_gralloc. (Do not use on new
+* systems.) */
+ dri2_dpy->loader_extensions = droid_dri2_loader_extensions;
+ if (!dri2_load_driver(disp)) {
+ err = "DRI2: failed to load driver";
+ goto error;
+ }
+ #else
+ err = "DRI2: handle is not for a render node";
+ goto error;
+ #endif
+ } else {
+ dri2_dpy->loader_extensions = droid_image_loader_extensions;
+ if (!dri2_load_driver_dri3(disp)) {
+ err = "DRI3: failed to load driver";
+ goto error;
+ }
+}
+
+ return true;
+
+error:
+ free(dri2_dpy->driver_name);
+ dri2_dpy->driver_name = NULL;
+ return false;
+}
+
+static bool
+droid_probe_driver(int fd)
+{
+ char *driver_name;
+
+ driver_name = loader_get_driver_for_fd(fd);
+ if (driver_name == NULL)
+ return false;
+
+ free(driver_name);
+ return true;
+}
+
+typedef enum {
+ probe_error = -1,
+ probe_success = 0,
+ probe_filtered_out = 1,
+ probe_no_driver = 2
+} probe_ret_t;
+
+static probe_ret_t
+droid_probe_device(_EGLDisplay *disp, int fd, char *vendor)
+{
+ int ret;
+
+ drmVersionPtr ver = drmGetVersion(fd);
+ if (!ver)
+ return probe_error;
+
+ if (vendor != NULL && ver->name != NULL &&
+ strncmp(vendor, ver->name, PROPERTY_VALUE_MAX) != 0) {
+ ret = probe_filtered_out;
+ goto cleanup;
+ }
+
+
+ if (!droid_probe_driver(fd)) {
+ ret = probe_no_driver;
+ goto cleanup;
+ }
+
+ ret = probe_success;
+
+cleanup:
+ drmFreeVersion(ver);
+ return ret;
+}
+
+static int
+droid_open_device(_EGLDisplay *disp)
+{
+ const int MAX_DRM_DEVICES = 32;
+ int prop_set, num_devices;
+ int fd = -1, fallback_fd = -1;
+
+ char *vendor_name = NULL;
+ char vendor_buf[PROPERTY_VALUE_MAX];
+ if (property_get("drm.gpu.vendor_name", vendor_buf, NULL) > 0);
+ vendor_name = vendor_buf;
+
+ const char *drm_dir_name = "/
