[Piglit] [v5 02/12] framework: hardware independent interface for dma buffers

[Chad Versace]

On 05/07/2013 02:13 PM, Chad Versace wrote:
> On 05/03/2013 04:26 AM, 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.
>>
>> v2 (Chad):
>>     - replace 'void*' buffer type by 'struct piglit_dma_buf *'
>>
>> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>> ---
>>   tests/util/piglit-framework-gl.c                   | 24 ++++++++++++++++++++++
>>   tests/util/piglit-framework-gl.h                   | 21 +++++++++++++++++++
>>   .../util/piglit-framework-gl/piglit_gl_framework.h |  9 ++++++++
>>   3 files changed, 54 insertions(+)
>>
>> diff --git a/tests/util/piglit-framework-gl.c b/tests/util/piglit-framework-gl.c
>> index 441e271..6ccc38d 100644
>> --- a/tests/util/piglit-framework-gl.c
>> +++ b/tests/util/piglit-framework-gl.c
>> @@ -162,3 +162,27 @@ 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,
>> +              struct piglit_dma_buf **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(struct piglit_dma_buf *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 4406c1b..26be68c 100644
>> --- a/tests/util/piglit-framework-gl.h
>> +++ b/tests/util/piglit-framework-gl.h
>> @@ -241,4 +241,25 @@ 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));
>>
>> +struct piglit_dma_buf;
>> +
>> +/**
>> + * 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,
>> +              struct piglit_dma_buf **buf, int *fd,
>> +              unsigned *stride, unsigned *offset);
>
>> +void
>> +piglit_destroy_dma_buf(struct piglit_dma_buf *buf);
>
> Is it safe to pass NULL to this function, like free()? Please document that.
>
> With that documentation, this patch is
> Reviewed-by: Chad Versace <chad.versace at linux.intel.com>

By the way, if in response to other review you need to make minor changes to this patch
(for example, changing the function signatures a little in response to Anholt's request),
then the custom among Mesa devs is to add a versioned Reviewed-by tag. This
patch is v2, so like this:

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