Topi Pohjolainen <topi.pohjolai...@intel.com> writes:
> diff --git a/tests/spec/ext_image_dma_buf_import/invalid_attributes.c
> b/tests/spec/ext_image_dma_buf_import/invalid_attributes.c
> new file mode 100644
> index 0000000..a6a6954
> --- /dev/null
> +++ b/tests/spec/ext_image_dma_buf_import/invalid_attributes.c
> @@ -0,0 +1,286 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "piglit-util-egl.h"
> +#define EGL_EGLEXT_PROTOTYPES 1
> +#include <EGL/eglext.h>
> +#include <unistd.h>
> +#include "ext_image_dma_buf_fourcc.h"
> +
> +/**
> + * @file invalid_attributes.c
> + *
> + * From the EXT_image_dma_buf_import spec:
> + *
> + * "* If <target> is EGL_LINUX_DMA_BUF_EXT and <buffer> is not NULL, the
> + * error EGL_BAD_PARAMETER is generated.
> + *
> + * * If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT
> + * attribute is set to a format not supported by the EGL, EGL_BAD_MATCH
> + * is generated.
> + *
> + * * If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT
> + * attribute indicates a single-plane format, EGL_BAD_ATTRIBUTE is
> + * generated if any of the EGL_DMA_BUF_PLANE1_* or EGL_DMA_BUF_PLANE2_*
> + * attributes are specified.
> + *
> + * * If <target> is EGL_LINUX_DMA_BUF_EXT and one or more of the values
> + * specified for a plane's pitch or offset isn't supported by EGL,
> + * EGL_BAD_ACCESS is generated."
> + */
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_es_version = 10;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static bool
> +test_excess_attributes(int fd, unsigned w, unsigned h, unsigned stride,
> + unsigned offset)
> +{
> + unsigned i;
> + const EGLint excess_attributes[][2 * 7 + 1] = {
> + { EGL_HEIGHT, w,
> + EGL_WIDTH, h,
> + EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
> + EGL_DMA_BUF_PLANE0_FD_EXT, fd,
> + EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
> + EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
> + EGL_DMA_BUF_PLANE1_FD_EXT, fd,
> + EGL_NONE },
> + { EGL_HEIGHT, w,
> + EGL_WIDTH, h,
> + EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
> + EGL_DMA_BUF_PLANE0_FD_EXT, fd,
> + EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
> + EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
> + EGL_DMA_BUF_PLANE1_OFFSET_EXT, 0,
> + EGL_NONE },
> + { EGL_HEIGHT, w,
> + EGL_WIDTH, h,
> + EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
> + EGL_DMA_BUF_PLANE0_FD_EXT, fd,
> + EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
> + EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
> + EGL_DMA_BUF_PLANE1_PITCH_EXT, stride,
> + EGL_NONE },
> + };
I think this particular subtest could be better done like the
test_invalid_hints, where a single attribute as a function parameter is
plugged into the stock array. It would clarify what's going on in the
test, and then you could easily do PLANE2_* checks, too.
> + for (i = 0; i < ARRAY_SIZE(excess_attributes); ++i) {
> + /**
> + * The spec says:
> + *
> + * "If <target> is EGL_LINUX_DMA_BUF_EXT, <dpy> must be a valid
> + * display, <ctx> must be EGL_NO_CONTEXT, and <buffer> must be
> + * NULL, cast into the type EGLClientBuffer."
> + */
Please remove this copypasta from the eglCreateImageKHR callsites that
aren't testing this particular spec text.
> + EGLImageKHR img = eglCreateImageKHR(eglGetCurrentDisplay(),
> + EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT,
> + (EGLClientBuffer)NULL, excess_attributes[i]);
> +
> + if (!piglit_check_egl_error(EGL_BAD_ATTRIBUTE)) {
> + if (img)
> + eglDestroyImageKHR(eglGetCurrentDisplay(), img);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +static bool
> +test_invalid_context(int fd, unsigned w, unsigned h, unsigned stride,
> + unsigned offset)
> +{
> + EGLImageKHR img;
> + EGLint attr[] = {
> + EGL_WIDTH, w,
> + EGL_HEIGHT, h,
> + EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
> + EGL_DMA_BUF_PLANE0_FD_EXT, fd,
> + EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
> + EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
> + EGL_NONE
> + };
> +
> + /**
> + * The spec says:
> + *
> + * "If <target> is EGL_LINUX_DMA_BUF_EXT, <dpy> must be a valid
> + * display, <ctx> must be EGL_NO_CONTEXT, and <buffer> must be
> + * NULL, cast into the type EGLClientBuffer."
> + */
> + img = eglCreateImageKHR(eglGetCurrentDisplay(), eglGetCurrentContext(),
> + EGL_LINUX_DMA_BUF_EXT, (EGLClientBuffer)NULL, attr);
> +
> + if (!piglit_check_egl_error(EGL_BAD_CONTEXT)) {
> + if (img)
> + eglDestroyImageKHR(eglGetCurrentDisplay(), img);
> + return false;
> + }
It's not explicitly called out in the spec, but I think this error case
would be EGL_BAD_PARAMETER like the other bad parameter cases for this
call.
> +
> +/**
> + * One re-uses the buffer for all the tests. Each test is expected to fail
> + * meaning that the ownership is not transferred to the EGL in any point.
> + */
"One re-uses" reads awkwardly. I think "We reuse" would be better.
> +enum piglit_result
> +piglit_display(void)
> +{
> + const unsigned char pixels[2 * 2 * 4];
> + struct piglit_dma_buf *buf;
> + unsigned stride;
> + unsigned offset;
> + int fd;
> + enum piglit_result res;
> + bool pass;
> +
> + res = piglit_create_dma_buf(2, 2, 4, pixels, 2 * 4, &buf, &fd, &stride,
> + &offset);
> + if (res != PIGLIT_PASS)
> + return res;
> +
> + pass = test_excess_attributes(fd, 2, 2, 2 * 4, 0);
> + pass = test_buffer_not_null(fd, 2, 2, 2 * 4, 0) && pass;
> + pass = test_invalid_context(fd, 2, 2, 2 * 4, 0) && pass;
> + pass = test_invalid_format(fd, 2, 2, 2 * 4, 0) && pass;
> + pass = test_pitch_zero(fd, 2, 2, 2 * 4, 0) && pass;
You should pass in the allocated stride from the piglit_create_dma_buf,
or for an implementation that aligns stride, you'll get BAD_ACCESS
errors by ignoring it. Also, please make variables "w" and "h" and use
them so I don't have to see this wall of 2s and 4s all over. The "4"
parameter in create_dma_buf was not obvious what it was doing (cpp, not
size)
pgpMQEFlLC4vk.pgp
Description: PGP signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
