[Mesa-dev] [PATCH] i965: Lift restriction in external textures for EGLImage support

[Tapani Pälli]

On 11/7/18 9:51 PM, Kenneth Graunke wrote:
> On Wednesday, October 31, 2018 5:12:40 PM PST Aditya Swarup wrote:
>> For Intel platforms, we support external textures only for EGLImages
>> created with EGL_EXT_image_dma_buf_import. This restriction seems to
>> be Intel specific and not present for other platforms.
>>
>> While running SKQP test - unitTest_EGLImageTest, GL_INVALID is sent
>> to the test because of this restriction.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105301
>> Fixes Skqp's unitTest_EGLImageTest test.
>>
>> Change-Id: I54a162db534d54858319fdb98ba502314d28fc27
>> Signed-off-by: Aditya Swarup <aditya.swarup at intel.com>
>> ---
>>   src/mesa/drivers/dri/i965/intel_image.h     |  3 ---
>>   src/mesa/drivers/dri/i965/intel_screen.c    |  2 --
>>   src/mesa/drivers/dri/i965/intel_tex_image.c | 10 ----------
>>   3 files changed, 15 deletions(-)
> 
> I'm pretty convinced by the spec quotes from Dongseong and Aditya
> in Bugzilla - I think this needs to be allowed.  Jason seems to think
> that we're already turning off compression.  I checked the OES_EGL_image
> spec and it discusses how mipmapping works - basically, if the client
> has any that aren't included in the EGL image, pitch them.  If the
> client later makes more, it ceases to be an EGL image target, and...
> you keep the broken pieces?  That seems pretty doable.
> 
> Jason and I are unsure whether we've handled all the corners, but it
> seems like a matter of raising errors in the right places...something
> we can probably do if we encounter issues later.
> 
> Tapani, what do you think?  It seems OK to me, and so I'd give this
> patch my Reviewed-by, but I want you to be happy with it before we
> land anything.

Android team has done full validation round and haven't seen any issues 
so it looks we are safe to lift this restriction.

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_image.h b/src/mesa/drivers/dri/i965/intel_image.h
>> index a8193c6def96..ca604159dc20 100644
>> --- a/src/mesa/drivers/dri/i965/intel_image.h
>> +++ b/src/mesa/drivers/dri/i965/intel_image.h
>> @@ -89,9 +89,6 @@ struct __DRIimageRec {
>>      GLuint tile_y;
>>      bool has_depthstencil;
>>   
>> -   /** The image was created with EGL_EXT_image_dma_buf_import. */
>> -   bool dma_buf_imported;
>> -
>>      /** Offset of the auxiliary compression surface in the bo. */
>>      uint32_t aux_offset;
>>   
>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
>> index b117928f18ec..26ec42b4c8b1 100644
>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> @@ -958,7 +958,6 @@ intel_dup_image(__DRIimage *orig_image, void *loaderPrivate)
>>      image->tile_y          = orig_image->tile_y;
>>      image->has_depthstencil = orig_image->has_depthstencil;
>>      image->data            = loaderPrivate;
>> -   image->dma_buf_imported = orig_image->dma_buf_imported;
>>      image->aux_offset      = orig_image->aux_offset;
>>      image->aux_pitch       = orig_image->aux_pitch;
>>   
>> @@ -1238,7 +1237,6 @@ intel_create_image_from_dma_bufs2(__DRIscreen *dri_screen,
>>         return NULL;
>>      }
>>   
>> -   image->dma_buf_imported = true;
>>      image->yuv_color_space = yuv_color_space;
>>      image->sample_range = sample_range;
>>      image->horizontal_siting = horizontal_siting;
>> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
>> index fae179214dd3..1b4083af530d 100644
>> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
>> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
>> @@ -616,16 +616,6 @@ intel_image_target_texture_2d(struct gl_context *ctx, GLenum target,
>>      if (image == NULL)
>>         return;
>>   
>> -   /* We support external textures only for EGLImages created with
>> -    * EGL_EXT_image_dma_buf_import. We may lift that restriction in the future.
>> -    */
>> -   if (target == GL_TEXTURE_EXTERNAL_OES && !image->dma_buf_imported) {
>> -      _mesa_error(ctx, GL_INVALID_OPERATION,
>> -            "glEGLImageTargetTexture2DOES(external target is enabled only "
>> -               "for images created with EGL_EXT_image_dma_buf_import");
>> -      return;
>> -   }
>> -
>>      /* Disallow depth/stencil textures: we don't have a way to pass the
>>       * separate stencil miptree of a GL_DEPTH_STENCIL texture through.
>>       */
>>
>