[Mesa-dev] [v7 7/9] intel: restrict dma-buf-import images to external sampling only

[Chad Versace]

On 07/10/2013 01:24 AM, Topi Pohjolainen wrote:
> Memory originating outside mesa stack is meant to be for reading
> only. In addition, the restrictions imposed by the image external
> extension should apply. For example, users shouldn't be allowed
> to generare mip-trees based on these images.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>   src/mesa/drivers/dri/i965/intel_fbo.c       | 7 +++++++
>   src/mesa/drivers/dri/i965/intel_regions.h   | 8 +++++++-
>   src/mesa/drivers/dri/i965/intel_screen.c    | 1 +
>   src/mesa/drivers/dri/i965/intel_tex_image.c | 9 +++++++++
>   4 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 25024fb..15f6130 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -265,6 +265,13 @@ intel_image_target_renderbuffer_storage(struct gl_context *ctx,
>      if (image->planar_format && image->planar_format->nplanes > 1)
>         return;
>
> +   /* Buffers originating from outside are for read-only. */
> +   if (image->dma_buf_imported) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +            "glEGLImageTargetRenderbufferStorage(dma buffers are read-only)");
> +      return;
> +   }
> +
>      /* __DRIimage is opaque to the core so it has to be checked here */
>      switch (image->format) {
>      case MESA_FORMAT_RGBA8888_REV:
> diff --git a/src/mesa/drivers/dri/i965/intel_regions.h b/src/mesa/drivers/dri/i965/intel_regions.h
> index 385d4b2..13e9e80 100644
> --- a/src/mesa/drivers/dri/i965/intel_regions.h
> +++ b/src/mesa/drivers/dri/i965/intel_regions.h
> @@ -150,7 +150,13 @@ struct __DRIimageRec {
>      GLuint tile_y;
>      bool has_depthstencil;
>
> -   /* Provided by dma_buf import extension */
> +   /*
> +    * Provided by dma_buf import extension. The flag is set in order to
> +    * restrict the use of the image later on.
> +    *
> +    * See intel_image_target_texture_2d()
> +    */


Same thing as before, please use the full extension name.

Also, as explained in review to patch 6, the multi-line comment
is unconventionally formatted. This should be a Doxygen comment,
which start with '/**'.


> +   bool dma_buf_imported;
>      enum __DRIYUVColorSpace yuv_color_space;
>      enum __DRISampleRange sample_range;
>      enum __DRIChromaSiting horizontal_siting;
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 2cec04f..ceeae3f 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -719,6 +719,7 @@ intel_create_image_from_dma_bufs(__DRIscreen *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 9c5d234..c2912b6 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -367,6 +367,15 @@ intel_image_target_texture_2d(struct gl_context *ctx, GLenum target,
>      if (image == NULL)
>         return;
>
> +   /*
> +    * Images originating via the dma-buffer-import extension can be used only
> +    * with the image-external extension.
> +    */
> +   if (image->dma_buf_imported && target != GL_TEXTURE_EXTERNAL_OES) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION, __func__);
> +      return;
> +   }
> +

Again, comment formatting.

Also, please use the full extension names, EGL_EXT_image_dma_buf_import and
GL_OES_EGL_image_external, to aid in grepping the code.

Also, since _mesa_error() let's us give the user an informative error message, let's do
that.