Hello Jim,
On 28 July 2015 at 02:57, Bish, Jim <jim.b...@intel.com> wrote:
> From: Jim Bish <jim.b...@intel.com>
>
Would you mind splitting this into separate patches ? Adding a few
words in the commit log(s) would be highly preferable.
> ---
> Android.common.mk | 10 ++++++++++
> Android.mk | 3 +++
> src/mesa/Android.libmesa_dricore.mk | 2 +-
> src/mesa/drivers/dri/i965/intel_fbo.c | 6 +++++-
> src/mesa/drivers/dri/i965/intel_tex_image.c | 4 +++-
> 5 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/Android.common.mk b/Android.common.mk
> index d662d60..68c2e1b 100644
> --- a/Android.common.mk
> +++ b/Android.common.mk
> @@ -76,6 +76,16 @@ LOCAL_CFLAGS += \
> -D__STDC_LIMIT_MACROS
> endif
>
> +# add libdrm if there are hardware drivers
> +ifneq ($(filter-out swrast,$(MESA_GPU_DRIVERS)),)
> +LOCAL_CFLAGS += -DHAVE_LIBDRM
> +LOCAL_CFLAGS += \
> + -I$(DRM_TOP)/include/drm \
> + -I$(DRM_TOP) \
> + -I$(DRM_GRALLOC_TOP)
The above includes should _not_ be needed. Please make sure that
LOCAL_EXPORT_C_INCLUDE_DIRS for libdrm and friends are set correctly.
The former two are addressed upstream [1] while there is a patch for
the last one on mesa-dev (as there is no upstream for drm_gralloc) [2]
Do you know of any plans to use upstream libdrm ? Or at least
rebase/merge with upstream changes ?
> +LOCAL_SHARED_LIBRARIES += libdrm
> +endif
> +
The rest of the hunk seems ok, but please mention why in the commit log.
> LOCAL_CPPFLAGS += \
> $(if $(filter true,$(MESA_LOLLIPOP_BUILD)),-D_USING_LIBCXX) \
> -Wno-error=non-virtual-dtor \
> diff --git a/Android.mk b/Android.mk
> index ed160fb..8f523bb 100644
> --- a/Android.mk
> +++ b/Android.mk
> @@ -45,6 +45,9 @@ endif
> MESA_COMMON_MK := $(MESA_TOP)/Android.common.mk
> MESA_PYTHON2 := python
>
> +DRM_TOP := external/drm
> +DRM_GRALLOC_TOP := hardware/drm_gralloc
> +
Please don't. See above for details.
> classic_drivers := i915 i965
> gallium_drivers := swrast freedreno i915g ilo nouveau r300g r600g radeonsi
> vmwgfx vc4
>
> diff --git a/src/mesa/Android.libmesa_dricore.mk
> b/src/mesa/Android.libmesa_dricore.mk
> index 2e308b8..fef76c8 100644
> --- a/src/mesa/Android.libmesa_dricore.mk
> +++ b/src/mesa/Android.libmesa_dricore.mk
> @@ -50,7 +50,7 @@ endif # MESA_ENABLE_ASM
> ifeq ($(ARCH_X86_HAVE_SSE4_1),true)
> LOCAL_SRC_FILES += \
> main/streaming-load-memcpy.c \
> - mesa/main/sse_minmax.c
> + main/sse_minmax.c
> LOCAL_CFLAGS := \
> -msse4.1 \
> -DUSE_SSE41
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 87ba624..e6cdd76 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -355,13 +355,17 @@ intel_image_target_renderbuffer_storage(struct
> gl_context *ctx,
> screen->loaderPrivate);
> if (image == NULL)
> return;
> -
> +#ifndef HAVE_ANDROID_PLATFORM
> + /*
> + * Disable this check for Android
> + */
> if (image->planar_format && image->planar_format->nplanes > 1) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "glEGLImageTargetRenderbufferStorage(planar buffers are not "
> "supported as render targets.");
> return;
> }
> +#endif
Imho this should be a separate patch, with the commit and/or comment
providing more information.
>
> /* __DRIimage is opaque to the core so it has to be checked here */
> switch (image->format) {
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index 3611280..4b3e9cf 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -317,8 +317,10 @@ intel_image_target_texture_2d(struct gl_context *ctx,
> GLenum target,
> if (image == NULL)
> return;
>
> +#ifndef HAVE_ANDROID_PLATFORM
> /* We support external textures only for EGLImages created with
> * EGL_EXT_image_dma_buf_import. We may lift that restriction in the
> future.
> + * We have to remove this restriction for Android builds.
> */
> if (target == GL_TEXTURE_EXTERNAL_OES && !image->dma_buf_imported) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> @@ -326,7 +328,7 @@ intel_image_target_texture_2d(struct gl_context *ctx,
> GLenum target,
> "for images created with EGL_EXT_image_dma_buf_import");
> return;
> }
> -
> +#endif
Ditto.
Chad,
I was under the impression that both restrictions were lifted with
your EGL_EXT_image_dma_buf_import
/OES_EGL_image_external work. Would you know, off hand, what is still missing ?
Thanks
Emil
[1] http://cgit.freedesktop.org/mesa/drm/
[2] http://lists.freedesktop.org/archives/mesa-dev/2015-July/089333.html
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
