[Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds

[Pekka Paalanen]

On Fri,  8 Aug 2014 17:28:59 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> The EGL_EXT_image_dma_buf_import specification was revised (according to
> its revision history) on Dec 5th, 2013, for EGL to not take ownership of
> the file descriptors.
> 
> Do not close the file descriptors passed in to eglCreateImageKHR with
> EGL_LINUX_DMA_BUF_EXT target.
> 
> It is assumed, that the drivers, which ultimately process the file
> descriptors, do not close or modify them in any way either. This avoids
> the need to dup(), as it seems we would only need to just close the
> dup'd file descriptors right after.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
> 
> Hi,
> 
> the corresponding Piglit fix has already been sent to the piglit mailing
> list. Both this and that need to be applied to not regress Mesa' piglit run
> by one test (ext_image_dma_buf_import-ownership_transfer).
> 
> This patch fixes my test case on heavily modified Weston.

Sorry, I forgot. This patch would apply also to the 10.0, 10.1, and
10.2 stable branches. Should it be a candidate there?


Thanks,
pq

> ---
>  src/egl/drivers/dri2/egl_dri2.c | 37 ++++++-------------------------------
>  1 file changed, 6 insertions(+), 31 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 5602ec3..cd85fd3 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
>  /**
>   * The spec says:
>   *
> - * "If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
> - *  the EGL takes ownership of the file descriptor and is responsible for
> - *  closing it, which it may do at any time while the EGLDisplay is
> - *  initialized."
> + * "If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target, the
> + *  EGL will take a reference to the dma_buf(s) which it will release at any
> + *  time while the EGLDisplay is initialized. It is the responsibility of the
> + *  application to close the dma_buf file descriptors."
> + *
> + * Therefore we must never close or otherwise modify the file descriptors.
>   */
> -static void
> -dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
> -{
> -   int already_closed[num_fds];
> -   unsigned num_closed = 0;
> -   unsigned i, j;
> -
> -   for (i = 0; i < num_fds; ++i) {
> -      /**
> -       * The same file descriptor can be referenced multiple times in case more
> -       * than one plane is found in the same buffer, just with a different
> -       * offset.
> -       */
> -      for (j = 0; j < num_closed; ++j) {
> -         if (already_closed[j] == fds[i])
> -            break;
> -      }
> -
> -      if (j == num_closed) {
> -         close(fds[i]);
> -         already_closed[num_closed++] = fds[i];
> -      }
> -   }
> -}
> -
>  static _EGLImage *
>  dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
>  			  EGLClientBuffer buffer, const EGLint *attr_list)
> @@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
>        return EGL_NO_IMAGE_KHR;
>  
>     res = dri2_create_image_from_dri(disp, dri_image);
> -   if (res)
> -      dri2_take_dma_buf_ownership(fds, num_fds);
>  
>     return res;
>  }