[igt-dev] [PATCH i-g-t v2 1/3] lib: Separated emit part of gpgpu_fill.

[Kalamarz, Lukasz]

On Fri, 2018-10-05 at 20:04 +0200, Tomasz Lis wrote:
> From: "Lis, Tomasz" <tomasz.lis at intel.com>
> 
> Having emitting and sumbission in one function was restricting
> possible use cases of the code. Now it is possible to emit
> the gpgpu_fill to a batchbuffer with something already there,
> or add more commands after.

Some other libs are using same approach (i.e. rendercopy, media_fill).
Maybe it will be a good idea to do this also for those libs

> 
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski at intel.com>
> 
> Signed-off-by: Tomasz Lis <tomasz.lis at intel.com>
> ---
>  lib/gpgpu_fill.c        | 59 ++++++++++++++++-----------------------
> ----------
>  lib/gpgpu_fill.h        |  8 +++----
>  lib/intel_batchbuffer.c | 12 +++++-----
>  lib/intel_batchbuffer.h |  2 +-
>  tests/gem_gpgpu_fill.c  | 19 ++++++++++++----
>  5 files changed, 45 insertions(+), 55 deletions(-)
> 

<SNIP>

> diff --git a/tests/gem_gpgpu_fill.c b/tests/gem_gpgpu_fill.c
> index dfb5816..d79f747 100644
> --- a/tests/gem_gpgpu_fill.c
> +++ b/tests/gem_gpgpu_fill.c
> @@ -44,6 +44,7 @@
>  #include <sys/time.h>
>  #include "drm.h"
>  #include "intel_bufmgr.h"
> +#include "gpu_cmds.h"
>  
>  #define WIDTH 64
>  #define HEIGHT 64
> @@ -99,7 +100,8 @@ igt_simple_main
>  	data_t data = {0, };
>  	struct intel_batchbuffer *batch = NULL;
>  	struct igt_buf dst;
> -	igt_fillfunc_t gpgpu_fill = NULL;
> +	igt_fillfunc_t emit_gpgpu_fill = NULL;
> +        uint32_t batch_end;

Whitespace

>  	int i, j;
>  
>  	data.drm_fd = drm_open_driver_render(DRIVER_INTEL);
> @@ -109,9 +111,9 @@ igt_simple_main
>  	data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
>  	igt_assert(data.bufmgr);
>  
> -	gpgpu_fill = igt_get_gpgpu_fillfunc(data.devid);
> +	emit_gpgpu_fill = igt_get_emit_gpgpu_fillfunc(data.devid);
>  
> -	igt_require_f(gpgpu_fill,
> +	igt_require_f(emit_gpgpu_fill,
>  		      "no gpgpu-fill function\n");
>  
>  	batch = intel_batchbuffer_alloc(data.bufmgr, data.devid);
> @@ -125,10 +127,19 @@ igt_simple_main
>  		}
>  	}
>  
> -	gpgpu_fill(batch,
> +        intel_batchbuffer_flush(batch);
> +
> +	emit_gpgpu_fill(batch,
>  		   &dst, 0, 0, WIDTH / 2, HEIGHT / 2,
>  		   COLOR_4C);
>  
> +        OUT_BATCH(MI_BATCH_BUFFER_END);
> +
> +        batch_end = intel_batchbuffer_align(batch, 8);
> +
> +        gen7_render_flush(batch, batch_end);
> +        intel_batchbuffer_reset(batch);
> +

Maybe move this part to lib as a new function (submit_gpgpu_fill)?
------
Lukasz

>  	for (i = 0; i < WIDTH; i++) {
>  		for (j = 0; j < HEIGHT; j++) {
>  			if (i < WIDTH / 2 && j < HEIGHT / 2)