On Fri, Aug 08, 2014 at 02:50:26PM +0300, Pekka Paalanen wrote: > From: Pekka Paalanen <[email protected]> > > 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. > > Update the ownership_transfer test to have the correct quote from the > latest specification (version 6), and invert the final test on close(). > > Now the test verifies, that after a successful import into EGL, followed > by guaranteed destruction of the EGLImage and the buffer reference > inside EGL, the dma_buf file descriptor still stays valid in the caller. > > Signed-off-by: Pekka Paalanen <[email protected]> > > --- > > I do not have commit access to piglit, and this is my first submission, > IIRC. > > This change causes Mesa to regress on this particular Piglit test. I am > planning to fix Mesa, too. > > I also didn't really understand the comment below the spec quote, so I > left it as is. It seems this test does not test at all that the buffer > stays valid in EGL after the creator side has been destroyed. This is > only about (not) keeping the dmabuf file descriptor open.
This is correct, and your change is the right thing to do. Reviewed-by: Topi Pohjolainen <[email protected]> Same as in case of the mesa patch, lets wait for other people to comment as well. > > Thanks, > pq > --- > tests/spec/ext_image_dma_buf_import/ownership_transfer.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/tests/spec/ext_image_dma_buf_import/ownership_transfer.c > b/tests/spec/ext_image_dma_buf_import/ownership_transfer.c > index ff51810..e3e34b6 100644 > --- a/tests/spec/ext_image_dma_buf_import/ownership_transfer.c > +++ b/tests/spec/ext_image_dma_buf_import/ownership_transfer.c > @@ -32,8 +32,9 @@ > * > * "3. Does ownership of the file descriptor pass to the EGL library? > * > - * ANSWER: If eglCreateImageKHR is successful, EGL assumes ownership of the > - * file descriptors and is responsible for closing them." > + * ANSWER: No, EGL does not take ownership of the file descriptors. It is > the > + * responsibility of the application to close the file descriptors on > success > + * and failure. > * > * > * Here one checks that the creator of the buffer can drop its reference once > @@ -99,8 +100,12 @@ test_create_and_destroy(unsigned w, unsigned h, void > *buf, int fd, > return PIGLIT_FAIL; > } > > - /* EGL stack should have closed the importing file descriptor by now */ > - return close(fd) && errno == EBADF ? PIGLIT_PASS : PIGLIT_FAIL; > + /* > + * EGL stack should have closed the importing file descriptor by now, Just drop this line here - "importing file descriptor" meant the very same fd we gave to the EGL stack (and the one we are about to close here). > + * but our own file descriptor must still be valid, and therefore > + * closing it must succeed. > + */ > + return close(fd) == 0 ? PIGLIT_PASS : PIGLIT_FAIL; > } > > enum piglit_result > -- > 1.8.5.5 > > _______________________________________________ > Piglit mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
