[Mesa-dev] [RFC 01/22] RFC: egl/x11: Support DRI3 v1.1

[Eric Engestrom]

On Thursday, 2017-06-08 19:43:59 +0100, Daniel Stone wrote:
> From: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
> 
> Add support for DRI3 v1.1, which allows pixmaps to be backed by
> multi-planar buffers, or those with format modifiers. This is both
> for allocating render buffers, as well as EGLImage imports from a
> native pixmap (EGL_NATIVE_PIXMAP_KHR).
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.c          |   3 +
>  src/egl/drivers/dri2/egl_dri2.h          |   1 +
>  src/egl/drivers/dri2/platform_x11_dri3.c |  62 +++++++-
>  src/glx/dri3_glx.c                       |  10 +-
>  src/loader/loader_dri3_helper.c          | 250 +++++++++++++++++++++++++------
>  src/loader/loader_dri3_helper.h          |  16 +-
>  6 files changed, 293 insertions(+), 49 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 7175e827c9..5b0799fcc4 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -804,6 +804,9 @@ dri2_setup_extensions(_EGLDisplay *disp)
>     if (!dri2_bind_extensions(dri2_dpy, mandatory_core_extensions, extensions, false))
>        return EGL_FALSE;
>  
> +   dri2_dpy->multibuffers_available &=
> +      (dri2_dpy->image && dri2_dpy->image->base.version >= 15);

Probably a matter of taste, but I find this more readable:

   if (!dri2_dpy->image || dri2_dpy->image->base.version < 15);
      dri2_dpy->multibuffers_available = false;


> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index 493a7f5218..3908512b9f 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
[snip]
> @@ -814,6 +820,27 @@ dri3_cpp_for_format(uint32_t format) {
>     }
>  }
>  
> +/* the DRIimage createImage function takes __DRI_IMAGE_FORMAT codes, while
> + * the createImageFromFds call takes __DRI_IMAGE_FOURCC codes. To avoid
> + * complete confusion, just deal in __DRI_IMAGE_FORMAT codes for now and
> + * translate to __DRI_IMAGE_FOURCC codes in the call to createImageFromFds
> + */
> +static int
> +image_format_to_fourcc(int format)
> +{
> +
> +   /* Convert from __DRI_IMAGE_FORMAT to __DRI_IMAGE_FOURCC (sigh) */
> +   switch (format) {
> +   case __DRI_IMAGE_FORMAT_SARGB8: return __DRI_IMAGE_FOURCC_SARGB8888;
> +   case __DRI_IMAGE_FORMAT_RGB565: return __DRI_IMAGE_FOURCC_RGB565;
> +   case __DRI_IMAGE_FORMAT_XRGB8888: return __DRI_IMAGE_FOURCC_XRGB8888;
> +   case __DRI_IMAGE_FORMAT_ARGB8888: return __DRI_IMAGE_FOURCC_ARGB8888;
> +   case __DRI_IMAGE_FORMAT_ABGR8888: return __DRI_IMAGE_FOURCC_ABGR8888;
> +   case __DRI_IMAGE_FORMAT_XBGR8888: return __DRI_IMAGE_FOURCC_XBGR8888;
> +   }
> +   return 0;
> +}
> +

I like to have "move code around" and other light refactor commits
separate from "change code" commits, but that's kind of a nitpick (and
I don't even always follow this rule myself, so feel free to ignore).


> @@ -890,25 +972,82 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable *draw, unsigned int format,
>           goto no_linear_buffer;
>     }
>  
> -   /* X wants the stride, so ask the image for it
> +   /* X want some information about the planes, so ask the image for it
>      */
> -   if (!draw->ext->image->queryImage(pixmap_buffer, __DRI_IMAGE_ATTRIB_STRIDE,
> -                                     &stride))
> -      goto no_buffer_attrib;
> -
> -   buffer->pitch = stride;
> +   draw->ext->image->queryImage(pixmap_buffer, __DRI_IMAGE_ATTRIB_NUM_PLANES,
> +                                &num_planes);
> +   if (num_planes <= 0)
> +      num_planes = 1;
> +
> +   for (i = 0; i < num_planes; i++) {
> +      __DRIimage *image = draw->ext->image->fromPlanar(pixmap_buffer, i, NULL);
> +      int ret;
> +      if (!image)
> +         image = pixmap_buffer;
> +      ret = draw->ext->image->queryImage(image, __DRI_IMAGE_ATTRIB_FD,
> +                                         &buffer_fds[i]);
> +      if (!ret) {
> +         if (image != pixmap_buffer)
> +            draw->ext->image->destroyImage(image);
> +         goto no_buffer_attrib;
> +      }
> +      ret = draw->ext->image->queryImage(image, __DRI_IMAGE_ATTRIB_STRIDE,
> +                                         &buffer->strides[i]);
> +      if (!ret) {
> +         if (image != pixmap_buffer)
> +            draw->ext->image->destroyImage(image);
> +         goto no_buffer_attrib;
[snip]
> @@ -930,6 +1069,8 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable *draw, unsigned int format,
>     return buffer;
>  
>  no_buffer_attrib:
> +   while (--i >= 0)
> +      close(buffer_fds[i]);

Unless my maths is wrong, this leaves the top fd open.

   do
   {
      close(buffer_fds[i]);
   }
   while (--i >= 0);

With this close() loop fix, this is
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

Cheers,
  Eric