[Mesa-dev] [PATCH v1 1/2] egl/android: #ifdef out flink name support

Robert Foss robert.foss at collabora.com
Tue May 1 08:13:34 UTC 2018


Hey Rob,

On 2018-05-01 04:20, Rob Herring wrote:
> On Fri, Apr 27, 2018 at 6:57 AM, Robert Foss <robert.foss at collabora.com> wrote:
>> From: Rob Herring <robh at 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 <robh at kernel.org>
>> Signed-off-by: Robert Foss <robert.foss at 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.


More information about the mesa-dev mailing list