On Wed, 13 Aug 2014 07:41:50 +0200 Gwenole Beauchesne <gb.de...@gmail.com> wrote:
> Hi, > > 2014-08-08 16:28 GMT+02:00 Pekka Paalanen <ppaala...@gmail.com>: > > From: Pekka Paalanen <pekka.paala...@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. > > Since this was a so radical change, shouldn't have change the API > string name instead to EXT_image_dma_buf_import2 for instance? It's like this in the Khronos registry, so how could I change the name? That is, I can write the code, but I need to know that is actually wanted and correct, and someone will do the same for the specs at Khronos. FWIW, when I wrote experimental dma_buf support in Weston, I was reading the Khronos spec. I just made a temporary hack to briefly be able to test on Mesa. And I know of a proprietary EGL implementation that implements this like the new spec, not like Mesa. I would like to consider this as just a Mesa bug, and a spec bug that was already fixed. > Anyway, could you please point to the following bug in your patch? > https://bugs.freedesktop.org/show_bug.cgi?id=76188 Done, thanks for the pointer. I also commented on the bug. After someone tells me whether or not I should add the stable CC's, I can send a new version. Thanks, pq > > Thanks. > > > 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. > > > > 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; > > } > > -- > > 1.8.5.5 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > Regards, _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
