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

From: "Kalamarz, Lukasz" <lukasz.kalamarz@intel.com>
To: "Lis, Tomasz" <tomasz.lis@intel.com>,
	"igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>
Cc: "Dunajski, Bartosz" <bartosz.dunajski@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t v2 1/3] lib: Separated emit part of gpgpu_fill.
Date: Wed, 24 Oct 2018 14:49:20 +0000	[thread overview]
Message-ID: <a5e03826e974bba75c33d29d3e563aa73b06cf84.camel@intel.com> (raw)
In-Reply-To: <1538762676-15259-2-git-send-email-tomasz.lis@intel.com>

On Fri, 2018-10-05 at 20:04 +0200, Tomasz Lis wrote:
> From: "Lis, Tomasz" <tomasz.lis@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@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> 
> Signed-off-by: Tomasz Lis <tomasz.lis@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)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev