[Piglit] [PATCH 1/3] framework: hardware independent interface for dma buffers

[Chad Versace]

On 04/16/2013 12:45 PM, Topi Pohjolainen wrote:
> In order to test the EXT_image_dma_buf_import, one needs a way for
> creating dma buffers that can be imported to EGL and filling them
> with data for the GL-stack to sample.
> While dma buffer themselves are only defined for linux, the actual
> writing of the buffers using CPU differs from hardware to another,
> and possibly from window system to another. The intention here is
> to push these details into the framework leaving the actual tests
> environment independent.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

This patch mostly looks good, and I like the approach you've taken:
making the utility functions independent of environment.

I have one suggestion. To enforce better type safety, I'd like to see void*
replaced by an opaque struct, named something like `struct piglit_dma_buf`.
But I don't want to block the series with that little nitpick, so this patch is

Reviewed-by: Chad Versace <chad.versace at linux.intel.com>

> ---
>   tests/util/piglit-framework-gl.c                   | 23 ++++++++++++++++++++++
>   tests/util/piglit-framework-gl.h                   | 18 +++++++++++++++++
>   .../util/piglit-framework-gl/piglit_gl_framework.h |  9 +++++++++
>   3 files changed, 50 insertions(+)
>
> diff --git a/tests/util/piglit-framework-gl.c b/tests/util/piglit-framework-gl.c
> index 441e271..d106839 100644
> --- a/tests/util/piglit-framework-gl.c
> +++ b/tests/util/piglit-framework-gl.c
> @@ -162,3 +162,26 @@ piglit_set_reshape_func(void (*func)(int w, int h))
>   	if (!gl_fw->set_reshape_func)
>   		gl_fw->set_reshape_func(gl_fw, func);
>   }
> +
> +enum piglit_result
> +piglit_create_dma_buf(unsigned w, unsigned h, unsigned cpp,
> +		      const void *src_data, unsigned src_stride,
> +		      void **buf, int *fd, unsigned *stride, unsigned *offset)
> +{
> +	*fd = 0;
> +	*stride = 0;
> +	*offset = 0;
> +
> +	if (!gl_fw->create_dma_buf)
> +		return PIGLIT_SKIP;
> +
> +	return gl_fw->create_dma_buf(w, h, cpp, src_data, src_stride, buf, fd,
> +				stride, offset);
> +}
> +
> +void
> +piglit_destroy_dma_buf(void *buf)
> +{
> +	if (gl_fw->destroy_dma_buf)
> +		gl_fw->destroy_dma_buf(buf);
> +}
> diff --git a/tests/util/piglit-framework-gl.h b/tests/util/piglit-framework-gl.h
> index bc3a3cd..aca2bbd 100644
> --- a/tests/util/piglit-framework-gl.h
> +++ b/tests/util/piglit-framework-gl.h
> @@ -254,4 +254,22 @@ void piglit_post_redisplay(void);
>   void piglit_set_keyboard_func(void (*func)(unsigned char key, int x, int y));
>   void piglit_set_reshape_func(void (*func)(int w, int h));
>
> +/**
> + * Create buffer suitable for dma_buf importing and set its contents to the
> + * given data (src_data). Different hardware may have different alignment
> + * constraints and hence one can specify one stride for the source and get
> + * another for the final buffer to be given further to EGL.
> + * An opaque handle, file descriptor, stride and offset for the buffer are only
> + * returned upon success indicated by the return value PIGLIT_PASS, otherwise
> + * no buffer is created. In case the framework simply does not support dma
> + * buffers, the return value is PIGLIT_SKIP instead of PIGLIT_FAIL.
> + */
> +enum piglit_result
> +piglit_create_dma_buf(unsigned w, unsigned h, unsigned cpp,
> +		      const void *src_data, unsigned src_stride,
> +		      void **buf, int *fd, unsigned *stride, unsigned *offset);
> +
> +void
> +piglit_destroy_dma_buf(void *buf);
> +
>   #endif /* PIGLIT_FRAMEWORK_H */
> diff --git a/tests/util/piglit-framework-gl/piglit_gl_framework.h b/tests/util/piglit-framework-gl/piglit_gl_framework.h
> index 6dbdf94..9535486 100644
> --- a/tests/util/piglit-framework-gl/piglit_gl_framework.h
> +++ b/tests/util/piglit-framework-gl/piglit_gl_framework.h
> @@ -71,6 +71,15 @@ struct piglit_gl_framework {
>
>   	void
>   	(*destroy)(struct piglit_gl_framework *gl_fw);
> +
> +	enum piglit_result
> +	(*create_dma_buf)(unsigned w, unsigned h, unsigned cpp,
> +			  const void *src_data, unsigned src_stride,
> +			  void **buf, int *fd, unsigned *stride,
> +			  unsigned *offset);
> +
> +	void
> +	(*destroy_dma_buf)(void *buf);
>   };
>
>   struct piglit_gl_framework*
>