Hey Rob,

On 2018-05-01 04:20, Rob Herring wrote:
On Fri, Apr 27, 2018 at 6:57 AM, Robert Foss <robert.f...@collabora.com> wrote:
From: Rob Herring <r...@kernel.org>

Maintaining both flink names and prime fd support which are provided by
2 different gralloc implementations is problematic because we have a
dependency on a specific gralloc implementation header.

This mostly disables the dependency on the gralloc implementation and
headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for
now, but the definition is added locally to remove the header
dependency.

drm_gralloc support can be enabled by setting
BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk.

Signed-off-by: Rob Herring <r...@kernel.org>
Signed-off-by: Robert Foss <robert.f...@collabora.com>
---
Changes since RFC:
  - Rebased on newer libdrm drmHandleMatch patch
  - Added support for driver probing

  src/egl/Android.mk                      |  6 ++++-
  src/egl/drivers/dri2/egl_dri2.h         |  2 --
  src/egl/drivers/dri2/platform_android.c | 41 +++++++++++++++++++++++++++++++--
  3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/src/egl/Android.mk b/src/egl/Android.mk
index 11818694f4..8412aeb798 100644
--- a/src/egl/Android.mk
+++ b/src/egl/Android.mk
@@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \
         libhardware \
         liblog \
         libcutils \
-       libgralloc_drm \
         libsync

+ifeq ($(BOARD_USES_DRM_GRALLOC),true)
+       LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC
+       LOCAL_SHARED_LIBRARIES += libgralloc_drm
+endif
+
  ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),)
  LOCAL_SHARED_LIBRARIES += libnativewindow
  endif
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index adabc527f8..5d8fbfa235 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1;

  #include <system/window.h>
  #include <hardware/gralloc.h>
-#include <gralloc_drm_handle.h>
-
  #endif /* HAVE_ANDROID_PLATFORM */

  #include "eglconfig.h"
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index 7f1a496ea2..ab1337f750 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -37,7 +37,11 @@
  #include "loader.h"
  #include "egl_dri2.h"
  #include "egl_dri2_fallbacks.h"
+
+#ifdef HAVE_DRM_GRALLOC
+#include <gralloc_drm_handle.h>
  #include "gralloc_drm.h"
+#endif /* HAVE_DRM_GRALLOC */

  #define ALIGN(val, align)      (((val) + (align) - 1) & ~((align) - 1))

@@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf)
     return (handle && handle->numFds) ? handle->data[0] : -1;
  }

+#ifdef HAVE_DRM_GRALLOC
  static int
  get_native_buffer_name(struct ANativeWindowBuffer *buf)
  {
     return gralloc_drm_get_gem_handle(buf->handle);
  }
+#endif /* HAVE_DRM_GRALLOC */

  static EGLBoolean
  droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
@@ -836,6 +842,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, 
_EGLContext *ctx,
     return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list);
  }

+#ifdef HAVE_DRM_GRALLOC
  static _EGLImage *
  droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx,
                               struct ANativeWindowBuffer *buf)
@@ -879,6 +886,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext 
*ctx,

     return &dri2_img->base;
  }
+#endif /* HAVE_DRM_GRALLOC */

  static EGLBoolean
  droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf,
@@ -935,7 +943,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp,
     if (fd >= 0)
        return droid_create_image_from_prime_fd(disp, ctx, buf, fd);

+#ifdef HAVE_DRM_GRALLOC
     return droid_create_image_from_name(disp, ctx, buf);
+#else
+   return NULL;
+#endif
  }

  static _EGLImage *
@@ -957,6 +969,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void 
*loaderPrivate)
  {
  }

+#ifdef HAVE_DRM_GRALLOC
  static int
  droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf,
                                      unsigned int *attachments, int count)
@@ -1032,6 +1045,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable,

     return dri2_surf->buffers;
  }
+#endif /* HAVE_DRM_GRALLOC */

  static unsigned
  droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap)
@@ -1114,6 +1128,14 @@ 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 = 0x40000002,
+};

Since you are keeping the header dependency, you can drop this hunk.

I'm a bit confused by this comment, which header dependency are you thinking of?

The gralloc_drm.h inclusion in platform_android.h is the only one I think I left in my patch, that you didn't have.


+
  static int
  droid_open_device(struct dri2_egl_display *dri2_dpy)
  {
@@ -1156,6 +1178,7 @@ static const struct dri2_egl_display_vtbl 
droid_display_vtbl = {
     .get_dri_drawable = dri2_surface_get_dri_drawable,
  };

+#ifdef HAVE_DRM_GRALLOC
  static const __DRIdri2LoaderExtension droid_dri2_loader_extension = {
     .base = { __DRI_DRI2_LOADER, 4 },

@@ -1164,6 +1187,7 @@ static const __DRIdri2LoaderExtension 
droid_dri2_loader_extension = {
     .getBuffersWithFormat = droid_get_buffers_with_format,
     .getCapability        = droid_get_capability,
  };
+#endif /* HAVE_DRM_GRALLOC */

  static const __DRIimageLoaderExtension droid_image_loader_extension = {
     .base = { __DRI_IMAGE_LOADER, 2 },
@@ -1173,12 +1197,14 @@ static const __DRIimageLoaderExtension 
droid_image_loader_extension = {
     .getCapability       = droid_get_capability,
  };

+#ifdef HAVE_DRM_GRALLOC
  static const __DRIextension *droid_dri2_loader_extensions[] = {
     &droid_dri2_loader_extension.base,
     &image_lookup_extension.base,
     &use_invalidate.base,
     NULL,
  };
+#endif /* HAVE_DRM_GRALLOC */

  static const __DRIextension *droid_image_loader_extensions[] = {
     &droid_image_loader_extension.base,
@@ -1228,20 +1254,31 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay 
*disp)

     dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == 
DRM_NODE_RENDER;

-   /* render nodes cannot use Gem names, and thus do not support
-    * the __DRI_DRI2_LOADER extension */
     if (!dri2_dpy->is_render_node) {
+#ifdef HAVE_DRM_GRALLOC
        dri2_dpy->loader_extensions = droid_dri2_loader_extensions;
        if (!dri2_load_driver(disp)) {
           err = "DRI2: failed to load driver";
           goto cleanup;
        }
     } else {
+      /* render nodes cannot use Gem names, and thus do not support
+       * the __DRI_DRI2_LOADER extension */
        dri2_dpy->loader_extensions = droid_image_loader_extensions;
        if (!dri2_load_driver_dri3(disp)) {
           err = "DRI3: failed to load driver";
           goto cleanup;
        }
+#else
+      err = "DRI2: handle is not for a render node";
+      goto cleanup;
+   }
+
+   dri2_dpy->loader_extensions = droid_image_loader_extensions;
+   if (!dri2_load_driver_dri3(disp)) {

Do we really need this twice?

Addressed as per Tomasz' comments.


Rob.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to