[Piglit] [RFC 2/3] framework: gbm: support for creating external buffers

[Chad Versace]

On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> Getting direct access to the gbm device through the waffle display
> and creating the buffer is rather straightforward thing to do.
> Writing to the buffer using CPU, however, requires priviledges for
> the underlying gem-ioctl.
> 
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  .../piglit-framework-gl/piglit_gbm_framework.c     |   78 ++++++++++++++++++++
>  1 file changed, 78 insertions(+)

It's clear here that you're trying to test a difficult thing and there is no
easy way to do it. I have several questions for you for which I don't know the
answer.


> diff --git a/tests/util/piglit-framework-gl/piglit_gbm_framework.c b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> index 4df3861..00e1425 100644
> --- a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> +++ b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> @@ -24,10 +24,18 @@
>  #include <assert.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> +#include <waffle.h>
> +#include <waffle_gbm.h>
> +#include <gbm.h>
> +#include <libdrm/drm.h>
> +#include <libdrm/i915_drm.h>
> +#include <sys/ioctl.h>
>  
>  #include "piglit-util-gl-common.h"
>  #include "piglit_gbm_framework.h"
>  
> +#define ALIGN(value, alignment) (((value) + alignment - 1) & ~(alignment - 1))
> +
>  static void
>  enter_event_loop(struct piglit_winsys_framework *winsys_fw)
>  {
> @@ -51,6 +59,73 @@ destroy(struct piglit_gl_framework *gl_fw)
>  	free(winsys_fw);
>  }
>  
> +static void *
> +map(struct gbm_device *gbm, struct gbm_bo *bo, unsigned n_bytes)
> +{
> +        int res;
> +        struct drm_i915_gem_mmap mmap_arg;
> +
> +        mmap_arg.handle = gbm_bo_get_handle(bo).u32;
> +        mmap_arg.offset = 0;
> +        mmap_arg.size = n_bytes;
> +
> +        res = ioctl(gbm_device_get_fd(gbm), DRM_IOCTL_I915_GEM_MMAP, &mmap_arg);
> +
> +        if (res)
> +                return 0;
> +
> +        return (void *)(uintptr_t)mmap_arg.addr_ptr;
> +}

This function is specific to i915, but gbm is specific only to drm. Is it possible
to rewrite this function using only drm ioctls? If it's not possbile, or if it that's a bad idea,
we should somehow detect here whether gbm is using i915 and skip the test if it's not.


> +
> +/* At the time of writing this, GBM did not have any alignment
> + * restrictions on height or width.
> + */
> +static void *
> +create_ext_420_buffer(struct piglit_gl_framework *gl_fw,
> +		      unsigned w, unsigned h, bool swap_vu,
> +		      const void *y, const void *u, const void *v)
> +{
> +	struct waffle_gbm_display *native = waffle_display_get_native(
> +		piglit_wfl_framework(gl_fw)->display)->gbm;
> +	struct gbm_bo *bo = gbm_bo_create(native->gbm_device, w, h,
> +				swap_vu ? GBM_FORMAT_YVU420 : GBM_FORMAT_YUV420,
> +				GBM_BO_USE_RENDERING);
> +	unsigned char *p;
> +	unsigned y_off = 0;
> +	unsigned u_off = ALIGN(w * h, 4096);
> +	unsigned v_off = u_off + ALIGN((w / 2) * (h / 2), 4096);
> +	unsigned total = v_off + ALIGN((w / 2) * (h / 2), 4096);

Why the 4096? Are you assuming that the buffer layout is aligned to 4096
because that is the size of a tile on Intel gpus?

I expected to see gbm_bo_get_stride() used in these calculations. Using it
may allow you to remove the hardcoded 4096.